You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ds...@apache.org on 2013/01/02 15:03:13 UTC
svn commit: r1427793 - in /lucene/dev/trunk/solr/core/src:
java/org/apache/solr/schema/AbstractSpatialFieldType.java
test/org/apache/solr/search/TestSolr4Spatial.java
Author: dsmiley
Date: Wed Jan 2 14:03:13 2013
New Revision: 1427793
URL: http://svn.apache.org/viewvc?rev=1427793&view=rev
Log:
SOLR-4231: AbstractSpatialFieldType extensibility, and throw 400 code for invalid shapes
Modified:
lucene/dev/trunk/solr/core/src/java/org/apache/solr/schema/AbstractSpatialFieldType.java
lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/TestSolr4Spatial.java
Modified: lucene/dev/trunk/solr/core/src/java/org/apache/solr/schema/AbstractSpatialFieldType.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/java/org/apache/solr/schema/AbstractSpatialFieldType.java?rev=1427793&r1=1427792&r2=1427793&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/java/org/apache/solr/schema/AbstractSpatialFieldType.java (original)
+++ lucene/dev/trunk/solr/core/src/java/org/apache/solr/schema/AbstractSpatialFieldType.java Wed Jan 2 14:03:13 2013
@@ -17,8 +17,12 @@ package org.apache.solr.schema;
* limitations under the License.
*/
+import com.google.common.base.Throwables;
+import com.google.common.cache.Cache;
+import com.google.common.cache.CacheBuilder;
import com.spatial4j.core.context.SpatialContext;
import com.spatial4j.core.context.SpatialContextFactory;
+import com.spatial4j.core.exception.InvalidShapeException;
import com.spatial4j.core.shape.Point;
import com.spatial4j.core.shape.Rectangle;
import com.spatial4j.core.shape.Shape;
@@ -45,7 +49,8 @@ import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.util.Map;
-import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutionException;
/**
* Abstract base class for Solr FieldTypes based on a Lucene 4 {@link SpatialStrategy}.
@@ -61,7 +66,7 @@ public abstract class AbstractSpatialFie
protected SpatialContext ctx;
protected SpatialArgsParser argsParser;
- private final ConcurrentHashMap<String, T> fieldStrategyMap = new ConcurrentHashMap<String,T>();
+ private final Cache<String, T> fieldStrategyCache = CacheBuilder.newBuilder().build();
@Override
protected void init(IndexSchema schema, Map<String, String> args) {
@@ -86,20 +91,20 @@ public abstract class AbstractSpatialFie
@Override
public final Field createField(SchemaField field, Object val, float boost) {
- throw new IllegalStateException("should be calling createFields because isPolyField() is true");
+ throw new IllegalStateException("instead call createFields() because isPolyField() is true");
}
@Override
- public final Field[] createFields(SchemaField field, Object val, float boost) {
+ public Field[] createFields(SchemaField field, Object val, float boost) {
String shapeStr = null;
Shape shape = null;
if (val instanceof Shape) {
shape = ((Shape) val);
} else {
shapeStr = val.toString();
- shape = ctx.readShape(shapeStr);
+ shape = parseShape(shapeStr);
}
- if( shape == null ) {
+ if (shape == null) {
log.debug("Field {}: null shape for input: {}", field, val);
return null;
}
@@ -131,6 +136,14 @@ public abstract class AbstractSpatialFie
}
}
+ protected Shape parseShape(String shapeStr) {
+ try {
+ return ctx.readShape(shapeStr);
+ } catch (InvalidShapeException e) {
+ throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, e);
+ }
+ }
+
protected String shapeToString(Shape shape) {
return ctx.toString(shape);
}
@@ -151,8 +164,8 @@ public abstract class AbstractSpatialFie
public Query getRangeQuery(QParser parser, SchemaField field, String part1, String part2, boolean minInclusive, boolean maxInclusive) {
if (!minInclusive || !maxInclusive)
throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Both sides of spatial range query must be inclusive: " + field.getName());
- Shape shape1 = ctx.readShape(part1);
- Shape shape2 = ctx.readShape(part2);
+ Shape shape1 = parseShape(part1);
+ Shape shape2 = parseShape(part2);
if (!(shape1 instanceof Point) || !(shape2 instanceof Point))
throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Both sides of spatial range query must be points: " + field.getName());
Point p1 = (Point) shape1;
@@ -165,14 +178,22 @@ public abstract class AbstractSpatialFie
@Override
public ValueSource getValueSource(SchemaField field, QParser parser) {
//This is different from Solr 3 LatLonType's approach which uses the MultiValueSource concept to directly expose
- // the an x & y pair of FieldCache value sources.
+ // the x & y pair of FieldCache value sources.
throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
"A ValueSource isn't directly available from this field. Instead try a query using the distance as the score.");
}
@Override
public Query getFieldQuery(QParser parser, SchemaField field, String externalVal) {
- return getQueryFromSpatialArgs(parser, field, argsParser.parse(externalVal, ctx));
+ return getQueryFromSpatialArgs(parser, field, parseSpatialArgs(externalVal));
+ }
+
+ protected SpatialArgs parseSpatialArgs(String externalVal) {
+ try {
+ return argsParser.parse(externalVal, ctx);
+ } catch (Exception e) {
+ throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, e);
+ }
}
private Query getQueryFromSpatialArgs(QParser parser, SchemaField field, SpatialArgs spatialArgs) {
@@ -208,18 +229,16 @@ public abstract class AbstractSpatialFie
* @return Non-null.
*/
public T getStrategy(final String fieldName) {
- T strategy = fieldStrategyMap.get(fieldName);
- //double-checked locking idiom
- if (strategy == null) {
- synchronized (fieldStrategyMap) {
- strategy = fieldStrategyMap.get(fieldName);
- if (strategy == null) {
- strategy = newSpatialStrategy(fieldName);
- fieldStrategyMap.put(fieldName,strategy);
+ try {
+ return fieldStrategyCache.get(fieldName, new Callable<T>() {
+ @Override
+ public T call() throws Exception {
+ return newSpatialStrategy(fieldName);
}
- }
+ });
+ } catch (ExecutionException e) {
+ throw Throwables.propagate(e.getCause());
}
- return strategy;
}
@Override
@@ -229,7 +248,8 @@ public abstract class AbstractSpatialFie
@Override
public SortField getSortField(SchemaField field, boolean top) {
- throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Sorting not supported on SpatialField: " + field.getName());
+ throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Sorting not supported on SpatialField: " + field.getName()+
+ ", instead try sorting by query.");
}
}
Modified: lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/TestSolr4Spatial.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/TestSolr4Spatial.java?rev=1427793&r1=1427792&r2=1427793&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/TestSolr4Spatial.java (original)
+++ lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/TestSolr4Spatial.java Wed Jan 2 14:03:13 2013
@@ -22,6 +22,7 @@ import com.carrotsearch.randomizedtestin
import com.spatial4j.core.context.SpatialContext;
import com.spatial4j.core.distance.DistanceUtils;
import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.SolrException;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Test;
@@ -60,6 +61,26 @@ public class TestSolr4Spatial extends So
assertU(commit());
}
+ @Test
+ public void testBadShapeParse400() {
+ assertQEx(null, req(
+ "fl", "id," + fieldName, "q", "*:*", "rows", "1000",
+ "fq", "{!field f="+fieldName+"}Intersects(NonexistentShape(89.9,-130 d=9))"), 400);
+ assertQEx(null, req(
+ "fl", "id," + fieldName, "q", "*:*", "rows", "1000",
+ "fq", "{!field f="+fieldName+"}Intersects(NonexistentShape(89.9,-130 d=9"), 400);//missing parens
+ assertQEx(null, req(
+ "fl", "id," + fieldName, "q", "*:*", "rows", "1000",
+ "fq", "{!field f="+fieldName+"}Intersectssss"), 400);
+
+ try {
+ assertU(adoc("id", "-1", fieldName, "NonexistentShape"));
+ fail();
+ } catch (SolrException e) {
+ assertEquals(400, e.code());
+ }
+ }
+
private void setupDocs() {
assertU(adoc("id", "1", fieldName, "32.7693246, -79.9289094"));
assertU(adoc("id", "2", fieldName, "33.7693246, -80.9289094"));