Fixed GeoIT. Cleaned up Query class
Harded search methods. Input validation was not done to ensure we have minimum data to execute queries. Project: http://git-wip-us.apache.org/repos/asf/incubator-usergrid/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-usergrid/commit/41647349 Tree: http://git-wip-us.apache.org/repos/asf/incubator-usergrid/tree/41647349 Diff: http://git-wip-us.apache.org/repos/asf/incubator-usergrid/diff/41647349 Branch: refs/heads/two-dot-o Commit: 41647349fe9735108a8ae99ca95d9c4448df9d30 Parents: de457e9 Author: Todd Nine <[email protected]> Authored: Tue Nov 11 14:42:15 2014 -0700 Committer: Todd Nine <[email protected]> Committed: Tue Nov 11 14:42:15 2014 -0700 ---------------------------------------------------------------------- .../corepersistence/CpRelationManager.java | 19 +++-- .../corepersistence/util/CpNamingUtils.java | 2 +- .../cassandra/RelationManagerImpl.java | 18 ++--- .../org/apache/usergrid/persistence/GeoIT.java | 9 +-- .../apache/usergrid/persistence/QueryTest.java | 15 ++++ stack/corepersistence/pom.xml | 2 +- .../usergrid/persistence/index/SearchTypes.java | 21 ++++- .../index/impl/EsEntityIndexImpl.java | 26 ++++-- .../usergrid/persistence/index/query/Query.java | 84 +++++++++++--------- stack/pom.xml | 7 -- stack/test-utils/pom.xml | 5 -- 11 files changed, 125 insertions(+), 83 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-usergrid/blob/41647349/stack/core/src/main/java/org/apache/usergrid/corepersistence/CpRelationManager.java ---------------------------------------------------------------------- diff --git a/stack/core/src/main/java/org/apache/usergrid/corepersistence/CpRelationManager.java b/stack/core/src/main/java/org/apache/usergrid/corepersistence/CpRelationManager.java index 561c269..9a729b8 100644 --- a/stack/core/src/main/java/org/apache/usergrid/corepersistence/CpRelationManager.java +++ b/stack/core/src/main/java/org/apache/usergrid/corepersistence/CpRelationManager.java @@ -106,6 +106,7 @@ import org.apache.usergrid.utils.IndexUtils; import org.apache.usergrid.utils.MapUtils; import org.apache.usergrid.utils.UUIDUtils; +import com.google.common.base.Preconditions; import com.yammer.metrics.annotation.Metered; import me.prettyprint.hector.api.Keyspace; @@ -418,7 +419,7 @@ public class CpRelationManager implements RelationManager { } else { - String connName = CpNamingUtils.getCollectionName( edge.getType() ); + String connName = CpNamingUtils.getConnectionType( edge.getType() ); indexScope = new IndexScopeImpl( new SimpleId( sourceEntity.getUuid(), sourceEntity.getType() ), CpNamingUtils.getConnectionScopeName( connName ) ); @@ -1432,14 +1433,20 @@ public class CpRelationManager implements RelationManager { @Override public Results searchConnectedEntities( Query query ) throws Exception { - if ( query == null ) { - query = new Query(); - } + Preconditions.checkNotNull(query, "query cannot be null"); + + final String connection = query.getConnectionType(); + + Preconditions.checkNotNull( connection, "connection must be specified" ); + +// if ( query == null ) { +// query = new Query(); +// } headEntity = em.validate( headEntity ); final IndexScope indexScope = new IndexScopeImpl( cpHeadEntity.getId(), - CpNamingUtils.getConnectionScopeName( query.getConnectionType() ) ); + CpNamingUtils.getConnectionScopeName( connection ) ); final SearchTypes searchTypes = SearchTypes.fromNullableTypes( query.getEntityType() ); @@ -1452,7 +1459,7 @@ public class CpRelationManager implements RelationManager { query = adjustQuery( query ); CandidateResults crs = ei.search( indexScope, searchTypes, query ); - return buildConnectionResults( query, crs, query.getConnectionType() ); + return buildConnectionResults( query, crs, connection ); } http://git-wip-us.apache.org/repos/asf/incubator-usergrid/blob/41647349/stack/core/src/main/java/org/apache/usergrid/corepersistence/util/CpNamingUtils.java ---------------------------------------------------------------------- diff --git a/stack/core/src/main/java/org/apache/usergrid/corepersistence/util/CpNamingUtils.java b/stack/core/src/main/java/org/apache/usergrid/corepersistence/util/CpNamingUtils.java index cf27203..9154752 100644 --- a/stack/core/src/main/java/org/apache/usergrid/corepersistence/util/CpNamingUtils.java +++ b/stack/core/src/main/java/org/apache/usergrid/corepersistence/util/CpNamingUtils.java @@ -66,7 +66,7 @@ public class CpNamingUtils { } - static public String getConnectionType( String edgeType ) { + static public String getConnectionType( String edgeType ) { String[] parts = edgeType.split( "\\|" ); return parts[1]; } http://git-wip-us.apache.org/repos/asf/incubator-usergrid/blob/41647349/stack/core/src/main/java/org/apache/usergrid/persistence/cassandra/RelationManagerImpl.java ---------------------------------------------------------------------- diff --git a/stack/core/src/main/java/org/apache/usergrid/persistence/cassandra/RelationManagerImpl.java b/stack/core/src/main/java/org/apache/usergrid/persistence/cassandra/RelationManagerImpl.java index 53bef80..be84176 100644 --- a/stack/core/src/main/java/org/apache/usergrid/persistence/cassandra/RelationManagerImpl.java +++ b/stack/core/src/main/java/org/apache/usergrid/persistence/cassandra/RelationManagerImpl.java @@ -80,11 +80,9 @@ import org.apache.usergrid.persistence.schema.CollectionInfo; import org.apache.usergrid.utils.IndexUtils; import org.apache.usergrid.utils.MapUtils; +import com.google.common.base.Preconditions; import com.yammer.metrics.annotation.Metered; -import me.prettyprint.cassandra.serializers.ByteBufferSerializer; -import me.prettyprint.cassandra.serializers.StringSerializer; -import me.prettyprint.cassandra.serializers.UUIDSerializer; import me.prettyprint.hector.api.Keyspace; import me.prettyprint.hector.api.beans.DynamicComposite; import me.prettyprint.hector.api.beans.HColumn; @@ -139,7 +137,7 @@ import static org.apache.usergrid.utils.InflectionUtils.singularize; import static org.apache.usergrid.utils.MapUtils.addMapSet; import static org.apache.usergrid.utils.UUIDUtils.getTimestampInMicros; import static org.apache.usergrid.utils.UUIDUtils.newTimeUUID; -import static org.apache.usergrid.persistence.cassandra.Serializers.*; + import org.apache.usergrid.persistence.entities.Application; import org.apache.usergrid.persistence.index.query.Query.Level; @@ -2037,12 +2035,14 @@ public class RelationManagerImpl implements RelationManager { @Metered(group = "core", name = "RelationManager_searchConnectedEntities") public Results searchConnectedEntities( Query query ) throws Exception { - if ( query == null ) { - query = new Query(); - } + Preconditions.checkNotNull(query, "Query must not be null"); + + + final String connectedEntityType = query.getEntityType(); + final String connectionType = query.getConnectionType(); - String connectedEntityType = query.getEntityType(); - String connectionType = query.getConnectionType(); + Preconditions.checkNotNull( connectedEntityType, "entityType must not be null" ); + Preconditions.checkNotNull( connectionType, "connectionType must not be null" ); headEntity = em.validate( headEntity ); http://git-wip-us.apache.org/repos/asf/incubator-usergrid/blob/41647349/stack/core/src/test/java/org/apache/usergrid/persistence/GeoIT.java ---------------------------------------------------------------------- diff --git a/stack/core/src/test/java/org/apache/usergrid/persistence/GeoIT.java b/stack/core/src/test/java/org/apache/usergrid/persistence/GeoIT.java index 7e4493c..39e39b2 100644 --- a/stack/core/src/test/java/org/apache/usergrid/persistence/GeoIT.java +++ b/stack/core/src/test/java/org/apache/usergrid/persistence/GeoIT.java @@ -200,11 +200,11 @@ public class GeoIT extends AbstractCoreIT { em.refreshIndex(); emSearchResults = em.searchConnectedEntities( user, - Query.fromQL( "location within 2000 of 37.776753, -122.407846" ) ); + Query.fromQL( "location within 2000 of 37.776753, -122.407846" ).setConnectionType( "likes" ) ); assertEquals( 1, emSearchResults.size() ); emSearchResults = em.searchConnectedEntities( user, - Query.fromQL( "location within 1000 of 37.776753, -122.407846" ) ); + Query.fromQL( "location within 1000 of 37.776753, -122.407846" ).setConnectionType( "likes" ) ); assertEquals( 0, emSearchResults.size() ); } @@ -380,10 +380,7 @@ public class GeoIT extends AbstractCoreIT { @Test public void testGeoWithIntersection() throws Exception { - UUID applicationId = setup.createApplication( "testOrganization", "testGeoWithIntersection" ); - assertNotNull( applicationId ); - - EntityManager em = setup.getEmf().getEntityManager( applicationId ); + EntityManager em = app.getEntityManager(); assertNotNull( em ); int size = 100; http://git-wip-us.apache.org/repos/asf/incubator-usergrid/blob/41647349/stack/core/src/test/java/org/apache/usergrid/persistence/QueryTest.java ---------------------------------------------------------------------- diff --git a/stack/core/src/test/java/org/apache/usergrid/persistence/QueryTest.java b/stack/core/src/test/java/org/apache/usergrid/persistence/QueryTest.java index 42489c3..e8cb7d4 100644 --- a/stack/core/src/test/java/org/apache/usergrid/persistence/QueryTest.java +++ b/stack/core/src/test/java/org/apache/usergrid/persistence/QueryTest.java @@ -128,6 +128,21 @@ public class QueryTest { assertEquals( 5, ( ( LongLiteral ) equal.getLiteral() ).getValue().intValue() ); } + @Test + public void withinDistanceCorrect(){ + final Query query = Query.fromQL( "location within 2000 of 37.776753, -122.407846" ); + + WithinOperand withinOperand = ( WithinOperand ) query.getRootOperand(); + + final float distance = withinOperand.getDistance().getFloatValue(); + final float lat = withinOperand.getLatitude().getFloatValue(); + final float lon = withinOperand.getLongitude().getFloatValue(); + + assertEquals( 2000f, distance, 0f ); + assertEquals( 37.776753f, lat, 0f ); + assertEquals( -122.407846f, lon, 0f ); + } + @Test public void testCodeEquals() { http://git-wip-us.apache.org/repos/asf/incubator-usergrid/blob/41647349/stack/corepersistence/pom.xml ---------------------------------------------------------------------- diff --git a/stack/corepersistence/pom.xml b/stack/corepersistence/pom.xml index 9bd53fa..b44fca7 100644 --- a/stack/corepersistence/pom.xml +++ b/stack/corepersistence/pom.xml @@ -59,7 +59,7 @@ limitations under the License. <commons.collections.version>3.2.1</commons.collections.version> <commons.io.version>2.4</commons.io.version> <commons.lang.version>3.1</commons.lang.version> - <elasticsearch.version>1.3.2</elasticsearch.version> + <elasticsearch.version>1.4.0</elasticsearch.version> <fasterxml-uuid.version>3.1.3</fasterxml-uuid.version> <guava.version>15.0</guava.version> <guice.version>4.0-beta5</guice.version> http://git-wip-us.apache.org/repos/asf/incubator-usergrid/blob/41647349/stack/corepersistence/queryindex/src/main/java/org/apache/usergrid/persistence/index/SearchTypes.java ---------------------------------------------------------------------- diff --git a/stack/corepersistence/queryindex/src/main/java/org/apache/usergrid/persistence/index/SearchTypes.java b/stack/corepersistence/queryindex/src/main/java/org/apache/usergrid/persistence/index/SearchTypes.java index b3eace5..35b3a8b 100644 --- a/stack/corepersistence/queryindex/src/main/java/org/apache/usergrid/persistence/index/SearchTypes.java +++ b/stack/corepersistence/queryindex/src/main/java/org/apache/usergrid/persistence/index/SearchTypes.java @@ -69,7 +69,7 @@ public class SearchTypes { */ public static SearchTypes fromNullableTypes(final String... types){ - if(types == null || types.length == 0){ + if(isEmpty(types) ){ return allTypes(); } @@ -77,6 +77,25 @@ public class SearchTypes { } + /** + * Return true if the array is empty, or it's elements contain a null + * @param input + * @return + */ + private static boolean isEmpty(final String[] input){ + if(input == null || input.length == 0){ + return true; + } + + for(int i = 0; i < input.length; i ++){ + if(input[i] == null){ + return true; + } + } + + return false; + } + @Override public boolean equals( final Object o ) { if ( this == o ) { http://git-wip-us.apache.org/repos/asf/incubator-usergrid/blob/41647349/stack/corepersistence/queryindex/src/main/java/org/apache/usergrid/persistence/index/impl/EsEntityIndexImpl.java ---------------------------------------------------------------------- diff --git a/stack/corepersistence/queryindex/src/main/java/org/apache/usergrid/persistence/index/impl/EsEntityIndexImpl.java b/stack/corepersistence/queryindex/src/main/java/org/apache/usergrid/persistence/index/impl/EsEntityIndexImpl.java index cb56c7d..a2c3090 100644 --- a/stack/corepersistence/queryindex/src/main/java/org/apache/usergrid/persistence/index/impl/EsEntityIndexImpl.java +++ b/stack/corepersistence/queryindex/src/main/java/org/apache/usergrid/persistence/index/impl/EsEntityIndexImpl.java @@ -59,6 +59,7 @@ import org.apache.usergrid.persistence.index.EntityIndexBatch; import org.apache.usergrid.persistence.index.IndexFig; import org.apache.usergrid.persistence.index.IndexScope; import org.apache.usergrid.persistence.index.SearchTypes; +import org.apache.usergrid.persistence.index.exceptions.IndexException; import org.apache.usergrid.persistence.index.query.CandidateResult; import org.apache.usergrid.persistence.index.query.CandidateResults; import org.apache.usergrid.persistence.index.query.Query; @@ -217,6 +218,12 @@ public class EsEntityIndexImpl implements EntityIndex { .setTemplate( config.getIndexPrefix() + "*" ).addMapping( "_default_", xcb ) // set mapping as the default for all types .execute().actionGet(); + + if(!pitr.isAcknowledged()){ + throw new IndexException( "Unable to create default mappings" ); + } + + } @@ -242,12 +249,6 @@ public class EsEntityIndexImpl implements EntityIndex { .setScroll( cursorTimeout + "m" ).setQuery( qb ); - if ( logger.isDebugEnabled() ) { - logger.debug( "Searching index {}\n scope{} \n type {}\n query {} limit {}", new Object[] { - this.indexName, context, entityTypes, qb.toString().replace( "\n", " " ), query.getLimit() - } ); - } - final FilterBuilder fb = query.createFilterBuilder(); @@ -296,11 +297,20 @@ public class EsEntityIndexImpl implements EntityIndex { logger.debug( " Sort: {} order by {}", booleanFieldName, order.toString() ); } + + + if ( logger.isDebugEnabled() ) { + logger.debug( "Searching index {}\n scope{} \n type {}\n query {} ", new Object[] { + this.indexName, context, entityTypes, srb + } ); + } + + try { searchResponse = srb.execute().actionGet(); } catch ( Throwable t ) { - logger.error( "Unable to communicate with elasticsearch" ); + logger.error( "Unable to communicate with elasticsearch", t ); failureMonitor.fail( "Unable to execute batch", t ); throw t; } @@ -325,7 +335,7 @@ public class EsEntityIndexImpl implements EntityIndex { searchResponse = ssrb.execute().actionGet(); } catch ( Throwable t ) { - logger.error( "Unable to communicate with elasticsearch" ); + logger.error( "Unable to communicate with elasticsearch", t ); failureMonitor.fail( "Unable to execute batch", t ); throw t; } http://git-wip-us.apache.org/repos/asf/incubator-usergrid/blob/41647349/stack/corepersistence/queryindex/src/main/java/org/apache/usergrid/persistence/index/query/Query.java ---------------------------------------------------------------------- diff --git a/stack/corepersistence/queryindex/src/main/java/org/apache/usergrid/persistence/index/query/Query.java b/stack/corepersistence/queryindex/src/main/java/org/apache/usergrid/persistence/index/query/Query.java index 07bee44..a057b02 100644 --- a/stack/corepersistence/queryindex/src/main/java/org/apache/usergrid/persistence/index/query/Query.java +++ b/stack/corepersistence/queryindex/src/main/java/org/apache/usergrid/persistence/index/query/Query.java @@ -91,7 +91,7 @@ public class Query { private Map<String, String> selectAssignments = new LinkedHashMap<String, String>(); private boolean mergeSelectResults = false; private Level level = Level.ALL_PROPERTIES; - private String connection; + private String connectionType; private List<String> permissions; private boolean reversed; private boolean reversedSet = false; @@ -125,7 +125,7 @@ public class Query { ? new LinkedHashMap<String, String>( q.selectAssignments ) : null; mergeSelectResults = q.mergeSelectResults; //level = q.level; - connection = q.connection; + connectionType = q.connectionType; permissions = q.permissions != null ? new ArrayList<String>( q.permissions ) : null; reversed = q.reversed; reversedSet = q.reversedSet; @@ -143,41 +143,47 @@ public class Query { } - public QueryBuilder createQueryBuilder(final String context) { - - - QueryBuilder queryBuilder; - - - /** - * Add our filter for context to our query for fast execution. Fast because it utilizes bitsets - * internally. See this post for more detail. - * http://www.elasticsearch.org/blog/all-about-elasticsearch-filter-bitsets/ - */ - + public QueryBuilder createQueryBuilder( final String context ) { + QueryBuilder queryBuilder = null; //we have a root operand. Translate our AST into an ES search if ( getRootOperand() != null ) { + //In the case of geo only queries, this will return null into the query builder. Once we start + //using tiles, we won't need this check any longer, since a geo query will return a tile query + post filter QueryVisitor v = new EsQueryVistor(); + try { getRootOperand().visit( v ); - - } catch ( IndexException ex ) { + } + catch ( IndexException ex ) { throw new RuntimeException( "Error building ElasticSearch query", ex ); } - // TODO evaluate performance when it's an all query. Do we need to put the context term first for performance? - queryBuilder = QueryBuilders.boolQuery().must( v.getQueryBuilder() ) - .must( QueryBuilders.termQuery( IndexingUtils.ENTITY_CONTEXT_FIELDNAME, context ) ); - } + + queryBuilder = v.getQueryBuilder(); + } + + + /** + * Add our filter for context to our query for fast execution. Fast because it utilizes bitsets + * internally. See this post for more detail. + * http://www.elasticsearch.org/blog/all-about-elasticsearch-filter-bitsets/ + */ + + + // TODO evaluate performance when it's an all query. Do we need to put the context term first for performance? + if ( queryBuilder != null ) { + queryBuilder = QueryBuilders.boolQuery().must( queryBuilder ).must( QueryBuilders + .termQuery( IndexingUtils.ENTITY_CONTEXT_FIELDNAME, context ) ); + } //nothing was specified ensure we specify the context in the search - else { + else { queryBuilder = QueryBuilders.termQuery( IndexingUtils.ENTITY_CONTEXT_FIELDNAME, context ); - } + } return queryBuilder; } @@ -283,7 +289,7 @@ public class Query { String ql = QueryUtils.queryStrFrom( params ); String type = ListUtils.first( params.get( "type" ) ); Boolean reversed = ListUtils.firstBoolean( params.get( "reversed" ) ); - String connection = ListUtils.first( params.get( "connection" ) ); + String connection = ListUtils.first( params.get( "connectionType" ) ); UUID start = ListUtils.firstUuid( params.get( "start" ) ); String cursor = ListUtils.first( params.get( "cursor" ) ); Integer limit = ListUtils.firstInteger( params.get( "limit" ) ); @@ -563,17 +569,6 @@ public class Query { this.type = type; } - - public String getConnectionType() { - return connection; - } - - - public void setConnectionType( String connection ) { - this.connection = connection; - } - - public List<String> getPermissions() { return permissions; } @@ -1373,18 +1368,29 @@ public class Query { } - public void setQl( String ql ) { + public Query setQl( String ql ) { this.ql = ql; + return this; } - public List<Identifier> getIdentifiers() { - return identifiers; + /** + * Get the connection type + * @return + */ + public String getConnectionType() { + return connectionType; } - public String getConnection() { - return connection; + /** + * Set the connection type + * @param connection + * @return + */ + public Query setConnectionType( final String connection ) { + this.connectionType = connection; + return this; } http://git-wip-us.apache.org/repos/asf/incubator-usergrid/blob/41647349/stack/pom.xml ---------------------------------------------------------------------- diff --git a/stack/pom.xml b/stack/pom.xml index ff70076..5a7bdd3 100644 --- a/stack/pom.xml +++ b/stack/pom.xml @@ -115,7 +115,6 @@ <tomcat-version>7.0.52</tomcat-version> <antlr.version>3.4</antlr.version> <tika.version>1.4</tika.version> - <elasticsearch.version>1.3.2</elasticsearch.version> <mockito.version>1.10.8</mockito.version> <usergrid.it.forkCount>3</usergrid.it.forkCount> @@ -1428,12 +1427,6 @@ </dependency> <dependency> - <groupId>org.elasticsearch</groupId> - <artifactId>elasticsearch</artifactId> - <version>${elasticsearch.version}</version> - </dependency> - - <dependency> <groupId>com.relayrides</groupId> <artifactId>pushy</artifactId> <!-- The sha in the version is the git commit used in this build. Check out the pushy source, then this commit to build the library locally --> http://git-wip-us.apache.org/repos/asf/incubator-usergrid/blob/41647349/stack/test-utils/pom.xml ---------------------------------------------------------------------- diff --git a/stack/test-utils/pom.xml b/stack/test-utils/pom.xml index f8e618d..96685f7 100644 --- a/stack/test-utils/pom.xml +++ b/stack/test-utils/pom.xml @@ -251,11 +251,6 @@ </dependency> <dependency> - <groupId>org.elasticsearch</groupId> - <artifactId>elasticsearch</artifactId> - </dependency> - - <dependency> <groupId>org.apache.tomcat.embed</groupId> <artifactId>tomcat-embed-core</artifactId> </dependency>
