From 8861934336c32343d57e20bfd54ac7efd5a82641 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Thu, 8 Mar 2012 11:39:41 +0100 Subject: [PATCH] DATAMONGO-360 - Fixed index information creation for geo indexes. Fixed a ClassCastException that occurred because we didn't consider index information of geo indexes (they return "2d" as direction). Introduced new IndexField abstraction that supersedes the fieldSpec Map in IndexInfo. --- .../mongodb/core/DefaultIndexOperations.java | 32 ++--- .../data/mongodb/core/index/IndexField.java | 132 ++++++++++++++++++ .../data/mongodb/core/index/IndexInfo.java | 72 +++++----- .../data/mongodb/core/MongoTemplateTests.java | 10 +- .../mongodb/core/geo/GeoSpatialTests.java | 24 ++++ .../core/index/IndexFieldUnitTests.java | 70 ++++++++++ 6 files changed, 288 insertions(+), 52 deletions(-) create mode 100644 spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/index/IndexField.java create mode 100644 spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/index/IndexFieldUnitTests.java diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/DefaultIndexOperations.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/DefaultIndexOperations.java index a9f3f7503..535092bbb 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/DefaultIndexOperations.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/DefaultIndexOperations.java @@ -16,14 +16,11 @@ package org.springframework.data.mongodb.core; import java.util.ArrayList; -import java.util.Iterator; -import java.util.LinkedHashMap; import java.util.List; -import java.util.Map; -import java.util.Map.Entry; import org.springframework.dao.DataAccessException; import org.springframework.data.mongodb.core.index.IndexDefinition; +import org.springframework.data.mongodb.core.index.IndexField; import org.springframework.data.mongodb.core.index.IndexInfo; import org.springframework.data.mongodb.core.query.Order; import org.springframework.util.Assert; @@ -123,25 +120,27 @@ public class DefaultIndexOperations implements IndexOperations { return getIndexData(dbObjectList); } - @SuppressWarnings("unchecked") private List getIndexData(List dbObjectList) { List indexInfoList = new ArrayList(); for (DBObject ix : dbObjectList) { - Map keyOrderMap = new LinkedHashMap(); DBObject keyDbObject = (DBObject) ix.get("key"); - Iterator entries = keyDbObject.toMap().entrySet().iterator(); + int numberOfElements = keyDbObject.keySet().size(); - while (entries.hasNext()) { - Entry thisEntry = (Entry) entries.next(); - String key = thisEntry.getKey().toString(); - int value = thisEntry.getValue(); - if (value == 1) { - keyOrderMap.put(key, Order.ASCENDING); - } else { - keyOrderMap.put(key, Order.DESCENDING); + List indexFields = new ArrayList(numberOfElements); + + for (String key : keyDbObject.keySet()) { + + Object value = keyDbObject.get(key); + + if (Integer.valueOf(1).equals(value)) { + indexFields.add(IndexField.create(key, Order.ASCENDING)); + } else if (Integer.valueOf(-1).equals(value)) { + indexFields.add(IndexField.create(key, Order.DESCENDING)); + } else if ("2d".equals(value)) { + indexFields.add(IndexField.geo(key)); } } @@ -151,12 +150,11 @@ public class DefaultIndexOperations implements IndexOperations { boolean dropDuplicates = ix.containsField("dropDups") ? (Boolean) ix.get("dropDups") : false; boolean sparse = ix.containsField("sparse") ? (Boolean) ix.get("sparse") : false; - indexInfoList.add(new IndexInfo(keyOrderMap, name, unique, dropDuplicates, sparse)); + indexInfoList.add(new IndexInfo(indexFields, name, unique, dropDuplicates, sparse)); } return indexInfoList; } }); } - } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/index/IndexField.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/index/IndexField.java new file mode 100644 index 000000000..11c78383d --- /dev/null +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/index/IndexField.java @@ -0,0 +1,132 @@ +/* + * Copyright 2012 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.mongodb.core.index; + +import org.springframework.data.mongodb.core.query.Order; +import org.springframework.util.Assert; +import org.springframework.util.ObjectUtils; + +/** + * Value object for an index field. + * + * @author Oliver Gierke + */ +public final class IndexField { + + private final String key; + private final Order order; + private final boolean isGeo; + + private IndexField(String key, Order order, boolean isGeo) { + + Assert.hasText(key); + Assert.isTrue(order != null ^ isGeo); + + this.key = key; + this.order = order; + this.isGeo = isGeo; + } + + /** + * Creates a default {@link IndexField} with the given key and {@link Order}. + * + * @param key must not be {@literal null} or emtpy. + * @param order must not be {@literal null}. + * @return + */ + public static IndexField create(String key, Order order) { + Assert.notNull(order); + return new IndexField(key, order, false); + } + + /** + * Creates a geo {@link IndexField} for the given key. + * + * @param key must not be {@literal null} or empty. + * @return + */ + public static IndexField geo(String key) { + return new IndexField(key, null, true); + } + + /** + * @return the key + */ + public String getKey() { + return key; + } + + /** + * Returns the order of the {@link IndexField} or {@literal null} in case we have a geo index field. + * + * @return the order + */ + public Order getOrder() { + return order; + } + + /** + * Returns whether the {@link IndexField} is a geo index field. + * + * @return the isGeo + */ + public boolean isGeo() { + return isGeo; + } + + /* + * (non-Javadoc) + * @see java.lang.Object#equals(java.lang.Object) + */ + @Override + public boolean equals(Object obj) { + + if (this == obj) { + return true; + } + + if (!(obj instanceof IndexField)) { + return false; + } + + IndexField that = (IndexField) obj; + + return this.key.equals(that.key) && ObjectUtils.nullSafeEquals(this.order, that.order) && this.isGeo == that.isGeo; + } + + /* + * (non-Javadoc) + * @see java.lang.Object#hashCode() + */ + @Override + public int hashCode() { + + int result = 17; + result += 31 * ObjectUtils.nullSafeHashCode(key); + result += 31 * ObjectUtils.nullSafeHashCode(order); + result += 31 * ObjectUtils.nullSafeHashCode(isGeo); + return result; + } + + /* + * (non-Javadoc) + * @see java.lang.Object#toString() + */ + @Override + public String toString() { + return String.format("IndexField [ key: %s, order: %s, isGeo: %s]", key, order, isGeo); + } +} diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/index/IndexInfo.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/index/IndexInfo.java index 6cb18bf2c..1ee4d0281 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/index/IndexInfo.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/index/IndexInfo.java @@ -15,33 +15,36 @@ */ package org.springframework.data.mongodb.core.index; -import java.util.Map; +import java.util.Collections; +import java.util.List; -import org.springframework.data.mongodb.core.query.Order; +import org.springframework.util.ObjectUtils; public class IndexInfo { - private final Map fieldSpec; + private final List indexFields; - private String name; + private final String name; + private final boolean unique; + private final boolean dropDuplicates; + private final boolean sparse; - private boolean unique = false; + public IndexInfo(List indexFields, String name, boolean unique, boolean dropDuplicates, boolean sparse) { - private boolean dropDuplicates = false; - - private boolean sparse = false; - - public IndexInfo(Map fieldSpec, String name, boolean unique, boolean dropDuplicates, boolean sparse) { - super(); - this.fieldSpec = fieldSpec; + this.indexFields = Collections.unmodifiableList(indexFields); this.name = name; this.unique = unique; this.dropDuplicates = dropDuplicates; this.sparse = sparse; } - public Map getFieldSpec() { - return fieldSpec; + /** + * Returns the individual index fields of the index. + * + * @return + */ + public List getIndexFields() { + return this.indexFields; } public String getName() { @@ -62,7 +65,7 @@ public class IndexInfo { @Override public String toString() { - return "IndexInfo [fieldSpec=" + fieldSpec + ", name=" + name + ", unique=" + unique + ", dropDuplicates=" + return "IndexInfo [indexFields=" + indexFields + ", name=" + name + ", unique=" + unique + ", dropDuplicates=" + dropDuplicates + ", sparse=" + sparse + "]"; } @@ -71,7 +74,7 @@ public class IndexInfo { final int prime = 31; int result = 1; result = prime * result + (dropDuplicates ? 1231 : 1237); - result = prime * result + ((fieldSpec == null) ? 0 : fieldSpec.hashCode()); + result = prime * result + ObjectUtils.nullSafeHashCode(indexFields); result = prime * result + ((name == null) ? 0 : name.hashCode()); result = prime * result + (sparse ? 1231 : 1237); result = prime * result + (unique ? 1231 : 1237); @@ -80,34 +83,39 @@ public class IndexInfo { @Override public boolean equals(Object obj) { - if (this == obj) + if (this == obj) { return true; - if (obj == null) + } + if (obj == null) { return false; - if (getClass() != obj.getClass()) + } + if (getClass() != obj.getClass()) { return false; + } IndexInfo other = (IndexInfo) obj; - if (dropDuplicates != other.dropDuplicates) + if (dropDuplicates != other.dropDuplicates) { return false; - if (fieldSpec == null) { - if (other.fieldSpec != null) + } + if (indexFields == null) { + if (other.indexFields != null) { return false; - } else if (!fieldSpec.equals(other.fieldSpec)) + } + } else if (!indexFields.equals(other.indexFields)) { return false; + } if (name == null) { - if (other.name != null) + if (other.name != null) { return false; - } else if (!name.equals(other.name)) + } + } else if (!name.equals(other.name)) { return false; - if (sparse != other.sparse) + } + if (sparse != other.sparse) { return false; - if (unique != other.unique) + } + if (unique != other.unique) { return false; + } return true; } - - /** - * [{ "v" : 1 , "key" : { "_id" : 1} , "ns" : "database.person" , "name" : "_id_"}, { "v" : 1 , "key" : { "age" : -1} - * , "ns" : "database.person" , "name" : "age_-1" , "unique" : true , "dropDups" : true}] - */ } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java index 482d137ee..06e59d737 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java @@ -46,8 +46,9 @@ import org.springframework.data.mongodb.MongoDbFactory; import org.springframework.data.mongodb.core.convert.CustomConversions; import org.springframework.data.mongodb.core.convert.MappingMongoConverter; import org.springframework.data.mongodb.core.index.Index; -import org.springframework.data.mongodb.core.index.Index.Duplicates; +import org.springframework.data.mongodb.core.index.IndexField; import org.springframework.data.mongodb.core.index.IndexInfo; +import org.springframework.data.mongodb.core.index.Index.Duplicates; import org.springframework.data.mongodb.core.mapping.MongoMappingContext; import org.springframework.data.mongodb.core.query.Criteria; import org.springframework.data.mongodb.core.query.Order; @@ -195,8 +196,11 @@ public class MongoTemplateTests { assertThat(ii.isUnique(), is(true)); assertThat(ii.isDropDuplicates(), is(true)); assertThat(ii.isSparse(), is(false)); - assertThat(ii.getFieldSpec().containsKey("age"), is(true)); - assertThat(ii.getFieldSpec().containsValue(Order.DESCENDING), is(true)); + + List indexFields = ii.getIndexFields(); + IndexField field = indexFields.get(0); + + assertThat(field, is(IndexField.create("age", Order.DESCENDING))); } @Test diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/geo/GeoSpatialTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/geo/GeoSpatialTests.java index 1959c5352..f44897658 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/geo/GeoSpatialTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/geo/GeoSpatialTests.java @@ -34,10 +34,14 @@ import org.springframework.context.ApplicationContext; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.dao.DataAccessException; import org.springframework.data.mongodb.core.CollectionCallback; +import org.springframework.data.mongodb.core.IndexOperations; import org.springframework.data.mongodb.core.MongoTemplate; import org.springframework.data.mongodb.core.Venue; import org.springframework.data.mongodb.core.index.GeospatialIndex; +import org.springframework.data.mongodb.core.index.IndexField; +import org.springframework.data.mongodb.core.index.IndexInfo; import org.springframework.data.mongodb.core.query.NearQuery; +import org.springframework.data.mongodb.core.query.Order; import org.springframework.data.mongodb.core.query.Query; import org.springframework.data.mongodb.monitor.ServerInfo; import org.springframework.expression.ExpressionParser; @@ -192,6 +196,26 @@ public class GeoSpatialTests { assertThat(indexInfo.get(1).get("ns").toString(), is("database.newyork")); } + /** + * @see DATAMONGO-360 + */ + @Test + public void indexInfoIsCorrect() { + + IndexOperations operations = template.indexOps(Venue.class); + List indexInfo = operations.getIndexInfo(); + + assertThat(indexInfo.size(), is(2)); + + List fields = indexInfo.get(0).getIndexFields(); + assertThat(fields.size(), is(1)); + assertThat(fields, hasItem(IndexField.create("_id", Order.ASCENDING))); + + fields = indexInfo.get(1).getIndexFields(); + assertThat(fields.size(), is(1)); + assertThat(fields, hasItem(IndexField.geo("location"))); + } + // TODO move to MongoAdmin public List getIndexInfo(Class clazz) { return template.execute(clazz, new CollectionCallback>() { diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/index/IndexFieldUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/index/IndexFieldUnitTests.java new file mode 100644 index 000000000..86cb4e64c --- /dev/null +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/index/IndexFieldUnitTests.java @@ -0,0 +1,70 @@ +/* + * Copyright 2012 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.mongodb.core.index; + +import static org.hamcrest.CoreMatchers.*; +import static org.junit.Assert.*; + +import org.junit.Test; +import org.springframework.data.mongodb.core.query.Order; + +/** + * Unit tests for {@link IndexField}. + * + * @author Oliver Gierke + */ +public class IndexFieldUnitTests { + + @Test + public void createsPlainIndexFieldCorrectly() { + + IndexField field = IndexField.create("foo", Order.ASCENDING); + + assertThat(field.getKey(), is("foo")); + assertThat(field.getOrder(), is(Order.ASCENDING)); + assertThat(field.isGeo(), is(false)); + } + + @Test + public void createsGeoIndexFieldCorrectly() { + + IndexField field = IndexField.geo("foo"); + + assertThat(field.getKey(), is("foo")); + assertThat(field.getOrder(), is(nullValue())); + assertThat(field.isGeo(), is(true)); + } + + @Test + public void correctEqualsForPlainFields() { + + IndexField first = IndexField.create("foo", Order.ASCENDING); + IndexField second = IndexField.create("foo", Order.ASCENDING); + + assertThat(first, is(second)); + assertThat(second, is(first)); + } + + @Test + public void correctEqualsForGeoFields() { + + IndexField first = IndexField.geo("bar"); + IndexField second = IndexField.geo("bar"); + + assertThat(first, is(second)); + assertThat(second, is(first)); + } +}