diff --git a/src/asciidoc/reference/bootstrap.adoc b/src/asciidoc/reference/bootstrap.adoc index d1865f07..3510db4c 100644 --- a/src/asciidoc/reference/bootstrap.adoc +++ b/src/asciidoc/reference/bootstrap.adoc @@ -106,11 +106,11 @@ Spring Data GemFire allows indices to be declared through the `index` element: ---- -Before creating an index, Spring Data GemFire will verify whether one with the same name already exists. If it does, -it will compare the properties and if they don't match, will remove the old one to create a new one. -If the properties match, Spring Data GemFire will simply return the index (in case it does not exist it will simply -create one). To prevent the update of the index, even if the properties do not match, set the property `override` -to false. +Before creating an Index, Spring Data GemFire will verify whether an Index with the same name already exists. +If an Index with the same name does exist, by default, SDG will "override" the existing Index by removing the old Index +first followed by creating a new Index with the same name based on the new definition, regardless if the old definition +was the same or not. To prevent the named Index definition change, especially when the old and new Index definitions +may not match, set the `override` property to false, which effectively returns the existing Index definition by name. Note that index declarations are not bound to a Region but rather are top-level elements (just like `gfe:cache`). This allows one to declare any number of indices on any Region whether they are just created or already exist diff --git a/src/main/java/org/springframework/data/gemfire/GemfireIndexException.java b/src/main/java/org/springframework/data/gemfire/GemfireIndexException.java index 8c011634..b275452d 100644 --- a/src/main/java/org/springframework/data/gemfire/GemfireIndexException.java +++ b/src/main/java/org/springframework/data/gemfire/GemfireIndexException.java @@ -25,30 +25,57 @@ import com.gemstone.gemfire.cache.query.IndexMaintenanceException; import com.gemstone.gemfire.cache.query.IndexNameConflictException; /** - * Gemfire-specific subclass thrown on index creation. + * Gemfire-specific subclass thrown on Index management. * * @author Costin Leau + * @author John Blum + * @see com.gemstone.gemfire.cache.query.IndexCreationException + * @see com.gemstone.gemfire.cache.query.IndexExistsException + * @see com.gemstone.gemfire.cache.query.IndexInvalidException + * @see com.gemstone.gemfire.cache.query.IndexMaintenanceException + * @see com.gemstone.gemfire.cache.query.IndexNameConflictException */ -@SuppressWarnings("serial") +@SuppressWarnings({ "serial", "unused" }) public class GemfireIndexException extends DataIntegrityViolationException { - public GemfireIndexException(IndexCreationException ex) { - super(ex.getMessage(), ex); + public GemfireIndexException(IndexCreationException cause) { + this(cause.getMessage(), cause); } - public GemfireIndexException(IndexExistsException ex) { - super(ex.getMessage(), ex); + public GemfireIndexException(String message, IndexCreationException cause) { + super(message, cause); } - public GemfireIndexException(IndexNameConflictException ex) { - super(ex.getMessage(), ex); + public GemfireIndexException(IndexExistsException cause) { + this(cause.getMessage(), cause); } - public GemfireIndexException(IndexMaintenanceException ex) { - super(ex.getMessage(), ex); + public GemfireIndexException(String message, IndexExistsException cause) { + super(message, cause); } - public GemfireIndexException(IndexInvalidException ex) { - super(ex.getMessage(), ex); + public GemfireIndexException(IndexInvalidException cause) { + this(cause.getMessage(), cause); } + + public GemfireIndexException(String message, IndexInvalidException cause) { + super(message, cause); + } + + public GemfireIndexException(IndexMaintenanceException cause) { + this(cause.getMessage(), cause); + } + + public GemfireIndexException(String message, IndexMaintenanceException cause) { + super(message, cause); + } + + public GemfireIndexException(IndexNameConflictException cause) { + this(cause.getMessage(), cause); + } + + public GemfireIndexException(String message, IndexNameConflictException cause) { + super(message, cause); + } + } diff --git a/src/main/java/org/springframework/data/gemfire/IndexFactoryBean.java b/src/main/java/org/springframework/data/gemfire/IndexFactoryBean.java index f063e5ce..f2ae594c 100644 --- a/src/main/java/org/springframework/data/gemfire/IndexFactoryBean.java +++ b/src/main/java/org/springframework/data/gemfire/IndexFactoryBean.java @@ -26,8 +26,8 @@ import org.springframework.beans.factory.FactoryBean; import org.springframework.beans.factory.InitializingBean; import org.springframework.beans.factory.config.ConfigurableBeanFactory; import org.springframework.data.gemfire.config.GemfireConstants; +import org.springframework.data.gemfire.util.CollectionUtils; import org.springframework.util.Assert; -import org.springframework.util.CollectionUtils; import org.springframework.util.ObjectUtils; import org.springframework.util.StringUtils; @@ -37,6 +37,7 @@ import com.gemstone.gemfire.cache.client.ClientCache; import com.gemstone.gemfire.cache.query.Index; import com.gemstone.gemfire.cache.query.IndexExistsException; import com.gemstone.gemfire.cache.query.IndexInvalidException; +import com.gemstone.gemfire.cache.query.IndexNameConflictException; import com.gemstone.gemfire.cache.query.IndexStatistics; import com.gemstone.gemfire.cache.query.QueryService; @@ -85,7 +86,7 @@ public class IndexFactoryBean implements InitializingBean, FactoryBean, B Assert.notNull(queryService, "QueryService is required to create an Index"); Assert.hasText(expression, "Index 'expression' is required"); - Assert.hasText(from, "Index 'from' clause is required"); + Assert.hasText(from, "Index 'from clause' is required"); if (IndexType.isKey(indexType)) { Assert.isNull(imports, "'imports' are not supported with a KEY Index"); @@ -150,7 +151,18 @@ public class IndexFactoryBean implements InitializingBean, FactoryBean, B } } catch (IndexExistsException e) { - return getExistingIndex(queryService, indexName); + throw new GemfireIndexException(String.format( + "An Index with a different name having the same definition as this Index (%1$s) already exists", + indexName), e); + } + catch (IndexNameConflictException e) { + // NOTE technically, the only way for an IndexNameConflictException to be thrown is if + // queryService.remove(existingIndex) above silently fails, since otherwise, when override is 'false', + // the existingIndex is already being returned. Given this state of affairs, an Index with the provided + // name is unresolvable based on what the user intended to happen, so just rethrow an Exception. + throw new GemfireIndexException(String.format( + "Failed to remove the existing Index%1$sbefore re-creating Index with name (%2$s)", + (override ? " on override " : " "), indexName), e); } catch (Exception e) { if (existingIndex != null) { @@ -231,7 +243,7 @@ public class IndexFactoryBean implements InitializingBean, FactoryBean, B /* (non-Javadoc) */ Index getExistingIndex(QueryService queryService, String indexName) { - for (Index index : queryService.getIndexes()) { + for (Index index : CollectionUtils.nullSafeCollection(queryService.getIndexes())) { if (index.getName().equalsIgnoreCase(indexName)) { return index; } @@ -404,7 +416,6 @@ public class IndexFactoryBean implements InitializingBean, FactoryBean, B } protected QueryService getQueryService() { - Assert.state(queryService != null, "The QueryService was not properly initialized!"); return queryService; } diff --git a/src/test/java/org/springframework/data/gemfire/IndexConflictsIntegrationTest.java b/src/test/java/org/springframework/data/gemfire/IndexConflictsIntegrationTest.java new file mode 100644 index 00000000..4b385729 --- /dev/null +++ b/src/test/java/org/springframework/data/gemfire/IndexConflictsIntegrationTest.java @@ -0,0 +1,285 @@ +/* + * Copyright 2010-2013 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.data.gemfire; + +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.instanceOf; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.not; +import static org.hamcrest.CoreMatchers.notNullValue; +import static org.hamcrest.CoreMatchers.sameInstance; +import static org.junit.Assert.assertThat; + +import java.util.Properties; + +import org.junit.Before; +import org.junit.Test; +import org.springframework.beans.factory.BeanCreationException; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.beans.factory.config.PropertyPlaceholderConfigurer; +import org.springframework.context.ConfigurableApplicationContext; +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.DependsOn; +import org.springframework.context.annotation.Import; + +import com.gemstone.gemfire.cache.Cache; +import com.gemstone.gemfire.cache.query.Index; +import com.gemstone.gemfire.cache.query.IndexExistsException; + +/** + * The IndexConflictsIntegrationTest class... + * + * @author John Blum + * @see org.junit.Test + * @link https://jira.spring.io/browse/SGF-432 + * @since 1.6.3 + */ +public class IndexConflictsIntegrationTest { + + protected void assertIndex(Index index, String expectedName, String expectedExpression, String expectedFromClause, + IndexType expectedType) { + + assertThat(index, is(notNullValue())); + assertThat(index.getName(), is(equalTo(expectedName))); + assertThat(index.getIndexedExpression(), is(equalTo(expectedExpression))); + assertThat(index.getFromClause(), is(equalTo(expectedFromClause))); + assertThat(IndexType.valueOf(index.getType()), is(equalTo(expectedType))); + } + + protected boolean close(ConfigurableApplicationContext applicationContext) { + if (applicationContext != null) { + applicationContext.close(); + return !(applicationContext.isActive() || applicationContext.isRunning()); + } + + return true; + } + + @Before + public void setup() { + System.getProperties().remove("gemfire.cache.region.index.override"); + + assertThat(System.getProperties().containsKey("gemfire.cache.region.index.override"), is(false)); + } + + @Test(expected = BeanCreationException.class) + public void indexDefinitionConflictThrowsException() { + ConfigurableApplicationContext applicationContext = null; + + try { + applicationContext = new AnnotationConfigApplicationContext( + IndexDefinitionConflictGemFireConfiguration.class); + } + catch (BeanCreationException expected) { + assertThat(expected.getMessage(), containsString("Error creating bean with name 'customerIdentityIndex'" + + " defined in org.springframework.data.gemfire.IndexConflictsIntegrationTest$IndexDefinitionConflictGemFireConfiguration:" + + " Invocation of init method failed")); + assertThat(expected.getCause(), is(instanceOf(GemfireIndexException.class))); + assertThat(expected.getCause().getMessage(), + containsString("An Index with a different name having the same definition" + + " as this Index (customerIdentityIndex) already exists")); + assertThat(expected.getCause().getCause(), is(instanceOf(IndexExistsException.class))); + assertThat(expected.getCause().getCause().getMessage(), is(equalTo("Similar Index Exists"))); + + throw expected; + } + finally { + assertThat(close(applicationContext), is(true)); + } + } + + @Test + public void indexNameConflictOverridesExistingIndex() { + ConfigurableApplicationContext applicationContext = null; + + try { + applicationContext = new AnnotationConfigApplicationContext(IndexNameConflictGemFireConfiguration.class); + + assertThat(applicationContext.getBeansOfType(Index.class).size(), is(equalTo(2))); + + Cache gemfireCache = applicationContext.getBean("gemfireCache", Cache.class); + + assertThat(gemfireCache.getQueryService().getIndexes().size(), is(equalTo(1))); + + Index customerLastNameIndex = applicationContext.getBean("customerLastNameIndex", Index.class); + + assertIndex(customerLastNameIndex, IndexNameConflictGemFireConfiguration.INDEX_NAME, + "lastName", "/Customers", IndexType.HASH); + + Index customerFirstNameIndex = applicationContext.getBean("customerFirstNameIndex", Index.class); + + assertIndex(customerFirstNameIndex, IndexNameConflictGemFireConfiguration.INDEX_NAME, + "firstName", "/Customers", IndexType.FUNCTIONAL); + + assertThat(customerFirstNameIndex, is(not(sameInstance(customerLastNameIndex)))); + assertThat(gemfireCache.getQueryService().getIndexes().iterator().next(), + is(sameInstance(customerFirstNameIndex))); + } + finally { + assertThat(close(applicationContext), is(true)); + } + } + + @Test + public void indexNameConflictReturnsExistingIndex() { + ConfigurableApplicationContext applicationContext = null; + + try { + System.setProperty("gemfire.cache.region.index.override", Boolean.FALSE.toString()); + + assertThat(System.getProperty("gemfire.cache.region.index.override", "true"), + is(equalTo(Boolean.FALSE.toString()))); + + applicationContext = new AnnotationConfigApplicationContext(IndexNameConflictGemFireConfiguration.class); + + assertThat(applicationContext.getBeansOfType(Index.class).size(), is(equalTo(2))); + + Cache gemfireCache = applicationContext.getBean("gemfireCache", Cache.class); + + assertThat(gemfireCache.getQueryService().getIndexes().size(), is(equalTo(1))); + + Index customerLastNameIndex = applicationContext.getBean("customerLastNameIndex", Index.class); + + assertIndex(customerLastNameIndex, IndexNameConflictGemFireConfiguration.INDEX_NAME, + "lastName", "/Customers", IndexType.HASH); + + Index customerFirstNameIndex = applicationContext.getBean("customerFirstNameIndex", Index.class); + + assertIndex(customerFirstNameIndex, IndexNameConflictGemFireConfiguration.INDEX_NAME, + "lastName", "/Customers", IndexType.HASH); + + assertThat(customerFirstNameIndex, is(sameInstance(customerLastNameIndex))); + assertThat(gemfireCache.getQueryService().getIndexes().iterator().next(), + is(sameInstance(customerLastNameIndex))); + } + finally { + System.getProperties().remove("gemfire.cache.region.index.override"); + + if (applicationContext != null) { + applicationContext.close(); + } + } + } + + @Configuration + @SuppressWarnings("unused") + public static class BaseGemFireConfiguration { + + @Bean + public Properties gemfireProperties() { + Properties gemfireProperties = new Properties(); + gemfireProperties.setProperty("name", IndexConflictsIntegrationTest.class.getSimpleName()); + gemfireProperties.setProperty("mcast-port", "0"); + gemfireProperties.setProperty("log-level", "warning"); + return gemfireProperties; + } + + @Bean + public CacheFactoryBean gemfireCache() { + CacheFactoryBean cacheFactoryBean = new CacheFactoryBean(); + cacheFactoryBean.setProperties(gemfireProperties()); + cacheFactoryBean.setUseBeanFactoryLocator(false); + return cacheFactoryBean; + } + + @Bean(name = "Customers") + public ReplicatedRegionFactoryBean customersRegion(Cache gemfireCache) { + ReplicatedRegionFactoryBean customersRegionFactory = new ReplicatedRegionFactoryBean(); + customersRegionFactory.setCache(gemfireCache); + customersRegionFactory.setName("Customers"); + customersRegionFactory.setPersistent(false); + return customersRegionFactory; + } + } + + @Configuration + @Import(BaseGemFireConfiguration.class) + @SuppressWarnings("unused") + public static class IndexDefinitionConflictGemFireConfiguration { + + @Bean + public IndexFactoryBean customerIdIndex(Cache gemfireCache) { + IndexFactoryBean indexFactoryBean = new IndexFactoryBean(); + indexFactoryBean.setCache(gemfireCache); + indexFactoryBean.setExpression("id"); + indexFactoryBean.setFrom("/Customers"); + indexFactoryBean.setType(IndexType.PRIMARY_KEY); + return indexFactoryBean; + } + + @Bean + @DependsOn("customerIdIndex") + public IndexFactoryBean customerIdentityIndex(Cache gemfireCache) { + IndexFactoryBean indexFactoryBean = new IndexFactoryBean(); + indexFactoryBean.setCache(gemfireCache); + indexFactoryBean.setExpression("id"); + indexFactoryBean.setFrom("/Customers"); + indexFactoryBean.setType(IndexType.PRIMARY_KEY); + return indexFactoryBean; + } + } + + @Configuration + @Import(BaseGemFireConfiguration.class) + @SuppressWarnings("unused") + public static class IndexNameConflictGemFireConfiguration { + + protected static final String INDEX_NAME = "CustomerNameIdx"; + + @Bean + public PropertyPlaceholderConfigurer propertyPlaceholderConfigurer() { + return new PropertyPlaceholderConfigurer(); + } + + @Bean + public IndexFactoryBean customerLastNameIndex(Cache gemfireCache, + @Value("${gemfire.cache.region.index.override:true}") boolean override) { + + IndexFactoryBean indexFactoryBean = new IndexFactoryBean(); + + indexFactoryBean.setCache(gemfireCache); + indexFactoryBean.setExpression("lastName"); + indexFactoryBean.setFrom("/Customers"); + indexFactoryBean.setName(INDEX_NAME); + indexFactoryBean.setOverride(override); + indexFactoryBean.setType(IndexType.HASH); + + return indexFactoryBean; + } + + @Bean + @DependsOn("customerLastNameIndex") + public IndexFactoryBean customerFirstNameIndex(Cache gemfireCache, + @Value("${gemfire.cache.region.index.override:true}") boolean override) { + + IndexFactoryBean indexFactoryBean = new IndexFactoryBean(); + + indexFactoryBean.setCache(gemfireCache); + indexFactoryBean.setExpression("firstName"); + indexFactoryBean.setFrom("/Customers"); + indexFactoryBean.setName(INDEX_NAME); + indexFactoryBean.setOverride(override); + indexFactoryBean.setType(IndexType.FUNCTIONAL); + + return indexFactoryBean; + } + } + +} diff --git a/src/test/java/org/springframework/data/gemfire/IndexFactoryBeanTest.java b/src/test/java/org/springframework/data/gemfire/IndexFactoryBeanTest.java index 21c1b410..a9a7ba82 100644 --- a/src/test/java/org/springframework/data/gemfire/IndexFactoryBeanTest.java +++ b/src/test/java/org/springframework/data/gemfire/IndexFactoryBeanTest.java @@ -16,6 +16,11 @@ package org.springframework.data.gemfire; +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.isA; +import static org.hamcrest.CoreMatchers.nullValue; +import static org.hamcrest.CoreMatchers.sameInstance; import static org.hamcrest.Matchers.startsWith; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -33,6 +38,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyZeroInteractions; import static org.mockito.Mockito.when; import java.util.ArrayList; @@ -44,7 +50,9 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; import org.junit.After; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; import org.springframework.beans.factory.BeanFactory; @@ -57,6 +65,7 @@ import com.gemstone.gemfire.cache.client.ClientCache; import com.gemstone.gemfire.cache.query.Index; import com.gemstone.gemfire.cache.query.IndexExistsException; import com.gemstone.gemfire.cache.query.IndexInvalidException; +import com.gemstone.gemfire.cache.query.IndexNameConflictException; import com.gemstone.gemfire.cache.query.IndexStatistics; import com.gemstone.gemfire.cache.query.QueryService; @@ -66,7 +75,6 @@ import com.gemstone.gemfire.cache.query.QueryService; * * @author John Blum * @see org.junit.Test - * @see org.springframework.beans.factory.BeanFactory * @see org.springframework.data.gemfire.IndexFactoryBean * @see com.gemstone.gemfire.cache.Cache * @see com.gemstone.gemfire.cache.client.ClientCache @@ -76,6 +84,9 @@ import com.gemstone.gemfire.cache.query.QueryService; */ public class IndexFactoryBeanTest { + @Rule + public ExpectedException expectedException = ExpectedException.none(); + private Cache mockCache = mock(Cache.class, "IndexFactoryBeanTest.MockCache"); private IndexFactoryBean indexFactoryBean = new IndexFactoryBean(); @@ -177,17 +188,16 @@ public class IndexFactoryBeanTest { } } - @Test(expected = IllegalArgumentException.class) + @Test public void afterPropertiesSetWithUnspecifiedFromClause() throws Exception { - try { - IndexFactoryBean indexFactoryBean = newIndexFactoryBean(); - indexFactoryBean.setExpression("id"); - indexFactoryBean.afterPropertiesSet(); - } - catch (IllegalArgumentException expected) { - assertEquals("Index 'from' clause is required", expected.getMessage()); - throw expected; - } + expectedException.expect(IllegalArgumentException.class); + expectedException.expectCause(is(nullValue(Throwable.class))); + expectedException.expectMessage("Index 'from clause' is required"); + + IndexFactoryBean indexFactoryBean = newIndexFactoryBean(); + + indexFactoryBean.setExpression("id"); + indexFactoryBean.afterPropertiesSet(); } @Test(expected = IllegalArgumentException.class) @@ -437,16 +447,48 @@ public class IndexFactoryBeanTest { verify(mockQueryService, times(1)).createIndex(eq("FunctionalIndex"), eq("someField"), eq("/Example")); } + @Test + public void createIndexOverridesExistingIndex() throws Exception { + Index mockExistingIndex = mock(Index.class, "createIndexOverridesExistingIndex.MockExistingIndex"); + Index mockOverridingIndex = mock(Index.class, "createIndexOverridesExistingIndex.MockOverridingIndex"); + + QueryService mockQueryService = mock(QueryService.class, "createIndexOverridesExistingIndex.MockQueryService"); + + when(mockExistingIndex.getName()).thenReturn("ExistingIndex"); + when(mockOverridingIndex.getName()).thenReturn("OverridingIndex"); + when(mockQueryService.getIndexes()).thenReturn(Collections.singletonList(mockExistingIndex)); + when(mockQueryService.createHashIndex(eq("ExistingIndex"), eq("someField"), eq("/Example"), + eq("example.DomainType"))).thenReturn(mockOverridingIndex); + + IndexFactoryBean indexFactoryBean = new IndexFactoryBean(); + + indexFactoryBean.setExpression("someField"); + indexFactoryBean.setFrom("/Example"); + indexFactoryBean.setImports("example.DomainType"); + indexFactoryBean.setName("OverridingIndex"); + indexFactoryBean.setType("HASH"); + + Index actualIndex = indexFactoryBean.createIndex(mockQueryService, "ExistingIndex"); + + assertSame(mockOverridingIndex, actualIndex); + + verifyZeroInteractions(mockOverridingIndex); + verify(mockExistingIndex, times(1)).getName(); + verify(mockQueryService, times(1)).removeIndex(same(mockExistingIndex)); + verify(mockQueryService, times(1)).createHashIndex(eq("ExistingIndex"), eq("someField"), eq("/Example"), + eq("example.DomainType")); + } + @Test public void createIndexReturnsExistingIndex() throws Exception { - Index mockExistingIndex = mock(Index.class, "createIndexReturnsExistingIndex.MockIndex.Existing"); - Index mockIndexTwo = mock(Index.class, "createIndexReturnsExistingIndex.MockIndex.Two"); + Index mockExistingIndex = mock(Index.class, "createIndexReturnsExistingIndex.MockExistingIndex"); + Index mockNewIndex = mock(Index.class, "createIndexReturnsExistingIndex.MockNewIndex"); QueryService mockQueryService = mock(QueryService.class, "createIndexReturnsExistingIndex.MockQueryService"); when(mockExistingIndex.getName()).thenReturn("ExistingIndex"); - when(mockIndexTwo.getName()).thenReturn("IndexTwo"); - when(mockQueryService.getIndexes()).thenReturn(Arrays.asList(mockExistingIndex, mockIndexTwo)); + when(mockNewIndex.getName()).thenReturn("NewIndex"); + when(mockQueryService.getIndexes()).thenReturn(Arrays.asList(mockExistingIndex, mockNewIndex)); IndexFactoryBean indexFactoryBean = new IndexFactoryBean(); @@ -456,84 +498,93 @@ public class IndexFactoryBeanTest { Index actualIndex = indexFactoryBean.createIndex(mockQueryService, "ExistingIndex"); assertSame(mockExistingIndex, actualIndex); + + verify(mockExistingIndex, times(1)).getName(); + verify(mockNewIndex, never()).getName(); + verify(mockQueryService, times(1)).getIndexes(); } @Test - public void createIndexReturnsExistingIndexAndThrowsIndexExistsExceptionOnCreate() throws Exception { + public void createIndexThrowsIndexNameConflictExceptionOnOverride() throws Exception { Index mockExistingIndex = mock(Index.class, - "createIndexReturnsExistingIndexAndThrowsIndexExistsExceptionOnCreate.MockIndex.Existing"); + "createIndexThrowsIndexNameConflictExceptionOnOverride.MockExistingIndex"); QueryService mockQueryService = mock(QueryService.class, - "createIndexReturnsExistingIndexAndThrowsIndexExistsExceptionOnCreate.MockQueryService"); + "createIndexThrowsIndexNameConflictExceptionOnOverride.MockQueryService"); when(mockExistingIndex.getName()).thenReturn("ExistingIndex"); when(mockQueryService.getIndexes()).thenReturn(Collections.singletonList(mockExistingIndex)); - when(mockQueryService.createKeyIndex(eq("ExistingIndex"), eq("id"), eq("/Example"))) - .thenThrow(new IndexExistsException("test")); - - IndexFactoryBean indexFactoryBean = new IndexFactoryBean(); - - indexFactoryBean.setExpression("id"); - indexFactoryBean.setFrom("/Example"); - indexFactoryBean.setName("ExistingIndex"); - indexFactoryBean.setOverride(true); - indexFactoryBean.setType("PRIMARY_KEY"); - - Index actualIndex = indexFactoryBean.createIndex(mockQueryService, "ExistingIndex"); - - assertSame(mockExistingIndex, actualIndex); - - verify(mockExistingIndex, times(2)).getName(); - verify(mockQueryService, times(2)).getIndexes(); - verify(mockQueryService, times(1)).removeIndex(same(mockExistingIndex)); - verify(mockQueryService, times(1)).createKeyIndex(eq("ExistingIndex"), eq("id"), eq("/Example")); - } - - @Test - public void createIndexOverridesExistingIndex() throws Exception { - Index mockExistingIndex = mock(Index.class, "createIndexOverridesExistingIndex.MockIndex.Existing"); - Index mockOverridingIndex = mock(Index.class, "createIndexOverridesExistingIndex.MockIndex.Overriding"); - - QueryService mockQueryService = mock(QueryService.class, "createIndexOverridesExistingIndex.MockQueryService"); - - when(mockExistingIndex.getName()).thenReturn("ExistingIndex"); - when(mockOverridingIndex.getName()).thenReturn("OverridingIndex"); - when(mockQueryService.getIndexes()).thenReturn(Collections.singletonList(mockExistingIndex)); - when(mockQueryService.createHashIndex(eq("ExistingIndex"), eq("someField"), eq("/Example"), - eq("org.example.DomainType"))).thenReturn(mockOverridingIndex); + when(mockQueryService.createIndex(any(String.class), any(String.class), any(String.class))) + .thenThrow(new IndexNameConflictException("TEST")); IndexFactoryBean indexFactoryBean = new IndexFactoryBean(); indexFactoryBean.setExpression("someField"); indexFactoryBean.setFrom("/Example"); - indexFactoryBean.setImports("org.example.DomainType"); - indexFactoryBean.setName("OverridingIndex"); - indexFactoryBean.setType("HASH"); + indexFactoryBean.setName("ExistingIndex"); + indexFactoryBean.setType("Functional"); - Index actualIndex = indexFactoryBean.createIndex(mockQueryService, "ExistingIndex"); + try { + expectedException.expect(GemfireIndexException.class); + expectedException.expectCause(isA(IndexNameConflictException.class)); + expectedException.expectMessage( + "Failed to remove the existing Index on override before re-creating Index with name (ExistingIndex)"); - assertSame(mockOverridingIndex, actualIndex); - - verify(mockQueryService, times(1)).getIndexes(); - verify(mockQueryService, times(1)).removeIndex(same(mockExistingIndex)); - verify(mockQueryService, times(1)).createHashIndex(eq("ExistingIndex"), eq("someField"), eq("/Example"), - eq("org.example.DomainType")); + indexFactoryBean.createIndex(mockQueryService, "ExistingIndex"); + } + finally { + verify(mockExistingIndex, times(1)).getName(); + verify(mockQueryService, times(1)).getIndexes(); + verify(mockQueryService, times(1)).createIndex(eq("ExistingIndex"), eq("someField"), eq("/Example")); + } } @Test - public void createIndexAddsExistingIndexOnException() throws Exception { - final Index mockExistingIndex = mock(Index.class, "createIndexAddsExistingIndexOnException.MockIndex.Existing"); - final Index mockIndexTwo = mock(Index.class, "createIndexAddsExistingIndexOnException.MockIndex.Two"); + public void createIndexThrowsIndexExistsException() throws Exception { + QueryService mockQueryService = mock(QueryService.class, + "createIndexThrowsIndexExistsException.MockQueryService"); + + when(mockQueryService.getIndexes()).thenReturn(null); + when(mockQueryService.createKeyIndex(eq("NewIndex"), eq("someField"), eq("/Example"))) + .thenThrow(new IndexExistsException("TEST")); + + IndexFactoryBean indexFactoryBean = new IndexFactoryBean(); + + indexFactoryBean.setExpression("someField"); + indexFactoryBean.setFrom("/Example"); + indexFactoryBean.setName("NewIndex"); + indexFactoryBean.setOverride(true); + indexFactoryBean.setType("PRIMARY_KEY"); + + try { + expectedException.expect(GemfireIndexException.class); + expectedException.expectCause(isA(IndexExistsException.class)); + expectedException.expectMessage( + "An Index with a different name having the same definition as this Index (NewIndex) already exists"); + + indexFactoryBean.createIndex(mockQueryService, "NewIndex"); + } + finally { + verify(mockQueryService, times(1)).getIndexes(); + verify(mockQueryService, never()).removeIndex(any(Index.class)); + verify(mockQueryService, times(1)).createKeyIndex(eq("NewIndex"), eq("someField"), eq("/Example")); + } + } + + @Test + public void createIndexAddsExistingIndexOnAnyException() throws Exception { + final Index mockExistingIndex = mock(Index.class, "createIndexAddsExistingIndexOnAnyException.MockExistingIndex"); + final Index mockIndexTwo = mock(Index.class, "createIndexAddsExistingIndexOnException.MockIndexTwo"); final List indexes = new ArrayList(1); indexes.add(mockIndexTwo); QueryService mockQueryService = mock(QueryService.class, - "createIndexAddsExistingIndexOnException.MockQueryService"); + "createIndexAddsExistingIndexOnAnyException.MockQueryService"); when(mockExistingIndex.getName()).thenReturn("ExistingIndex"); - when(mockIndexTwo.getName()).thenReturn("IndexTwo"); + when(mockIndexTwo.getName()).thenReturn("NewIndex"); when(mockQueryService.getIndexes()).then(new Answer>() { private boolean called = false; @@ -551,7 +602,7 @@ public class IndexFactoryBeanTest { }); when(mockQueryService.createIndex(eq("ExistingIndex"), eq("someField"), eq("/Example"))) - .thenThrow(new RuntimeException("test")); + .thenThrow(new RuntimeException("TEST")); IndexFactoryBean indexFactoryBean = new IndexFactoryBean(); @@ -566,9 +617,10 @@ public class IndexFactoryBeanTest { Index actualIndex = indexFactoryBean.createIndex(mockQueryService, "ExistingIndex"); - assertSame(mockExistingIndex, actualIndex); - assertEquals(2, indexes.size()); - assertTrue(indexes.contains(mockExistingIndex)); + assertThat(actualIndex, is(sameInstance(mockExistingIndex))); + assertThat(indexes.size(), is(equalTo(2))); + assertThat(indexes.contains(mockExistingIndex), is(true)); + assertThat(indexes.contains(mockIndexTwo), is(true)); verify(mockQueryService, times(3)).getIndexes(); verify(mockQueryService, times(1)).removeIndex(same(mockExistingIndex));