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 2016/06/07 20:49:43 UTC
lucene-solr:master: SOLR-8859: Fix AbstractSpatialFieldType to not
cycle through all Spatial4j provided formats. And Fix
RptWithGeometrySpatialField to be less brittle on init()
Repository: lucene-solr
Updated Branches:
refs/heads/master 66cd0edc5 -> fb37b3eb8
SOLR-8859: Fix AbstractSpatialFieldType to not cycle through all Spatial4j provided formats.
And Fix RptWithGeometrySpatialField to be less brittle on init()
Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/fb37b3eb
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/fb37b3eb
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/fb37b3eb
Branch: refs/heads/master
Commit: fb37b3eb8c4130c8b5f53e1741e9585743b26e4d
Parents: 66cd0ed
Author: David Smiley <ds...@apache.org>
Authored: Tue Jun 7 16:48:34 2016 -0400
Committer: David Smiley <ds...@apache.org>
Committed: Tue Jun 7 16:48:34 2016 -0400
----------------------------------------------------------------------
solr/CHANGES.txt | 4 +-
.../solr/schema/AbstractSpatialFieldType.java | 72 +++++++++-----------
.../schema/RptWithGeometrySpatialField.java | 11 +--
.../solr/schema/SpatialRPTFieldTypeTest.java | 32 ++++++---
4 files changed, 65 insertions(+), 54 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/fb37b3eb/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 56c111b..bed02f2 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -81,8 +81,8 @@ New Features
https://github.com/locationtech/spatial4j/blob/master/FORMATS.md
To return the FeatureCollection as the root element, add '&omitHeader=true" (ryan)
-* SOLR-8859: AbstractSpatialFieldType will now convert Shapes to/from Strings
- using the SpatialContext. (ryan)
+* SOLR-8859: Spatial fields like RPT can now be configured to use Spatial4j registered shape formats
+ e.g. via format="GeoJSON". (ryan, David Smiley)
* SOLR-445: new ToleranteUpdateProcessorFactory to support skipping update commands that cause
failures when sending multiple updates in a single request.
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/fb37b3eb/solr/core/src/java/org/apache/solr/schema/AbstractSpatialFieldType.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/schema/AbstractSpatialFieldType.java b/solr/core/src/java/org/apache/solr/schema/AbstractSpatialFieldType.java
index 3130004..12fcea3 100644
--- a/solr/core/src/java/org/apache/solr/schema/AbstractSpatialFieldType.java
+++ b/solr/core/src/java/org/apache/solr/schema/AbstractSpatialFieldType.java
@@ -26,9 +26,11 @@ import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.TreeSet;
-import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
+import com.google.common.base.Throwables;
+import com.google.common.cache.Cache;
+import com.google.common.cache.CacheBuilder;
import org.apache.lucene.document.Field;
import org.apache.lucene.document.StoredField;
import org.apache.lucene.index.IndexableField;
@@ -42,7 +44,6 @@ import org.apache.lucene.spatial.SpatialStrategy;
import org.apache.lucene.spatial.query.SpatialArgs;
import org.apache.lucene.spatial.query.SpatialArgsParser;
import org.apache.lucene.spatial.query.SpatialOperation;
-import org.apache.solr.common.SolrException.ErrorCode;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.params.SolrParams;
import org.apache.solr.response.TextResponseWriter;
@@ -64,10 +65,6 @@ import org.locationtech.spatial4j.shape.Shape;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import com.google.common.base.Throwables;
-import com.google.common.cache.Cache;
-import com.google.common.cache.CacheBuilder;
-
/**
* Abstract base class for Solr FieldTypes based on a Lucene 4 {@link SpatialStrategy}.
*
@@ -141,24 +138,21 @@ public abstract class AbstractSpatialFieldType<T extends SpatialStrategy> extend
}
final SupportedFormats fmts = ctx.getFormats();
- final String format = args.remove(FORMAT);
- if (format != null) {
- shapeWriter = fmts.getWriter(format);
- shapeReader = fmts.getReader(format);
- if(shapeWriter==null) {
- throw new SolrException(SolrException.ErrorCode.SERVER_ERROR,
- "Unknown Shape Format: "+ format);
- }
- if(shapeReader==null) {
- throw new SolrException(SolrException.ErrorCode.SERVER_ERROR,
- "Unknown Shape Format: "+ format);
- }
+ String format = args.remove(FORMAT);
+ if (format == null) {
+ format = "WKT";
+ }
+ shapeWriter = fmts.getWriter(format);
+ shapeReader = fmts.getReader(format);
+ if(shapeWriter==null) {
+ throw new SolrException(SolrException.ErrorCode.SERVER_ERROR,
+ "Unknown Shape Format: "+ format);
}
- else {
- // Otherwise, pick the first supported reader/writer
- shapeWriter = fmts.getWriters().get(0);
- shapeReader = fmts.getReaders().get(0);
+ if(shapeReader==null) {
+ throw new SolrException(SolrException.ErrorCode.SERVER_ERROR,
+ "Unknown Shape Format: "+ format);
}
+
argsParser = newSpatialArgsParser();
}
@@ -234,23 +228,29 @@ public abstract class AbstractSpatialFieldType<T extends SpatialStrategy> extend
/** Create a {@link Shape} from the input string */
public Shape parseShape(String str) {
+ str = str.trim();
if (str.length() == 0)
throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "empty string shape");
- Shape shape = null;
- if(shapeReader!=null) {
- shape = shapeReader.readIfSupported(str);
- }
-
- if(shape==null) {
- // Try all supported formats
- shape = ctx.getFormats().read(str);
+ // If the first char is promising, try to parse with SpatialUtils.parsePoint
+ char firstChar = str.charAt(0);
+ if (firstChar == '+' || firstChar == '-' || (firstChar >= '0' && firstChar <= '9')) {
+ try {
+ return SpatialUtils.parsePoint(str, ctx);
+ } catch (Exception e) {//ignore
+ }
}
- if(shape==null) {
- throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Unable to parse shape from: "+str);
+ try {
+ return shapeReader.read(str);
+ } catch (Exception e) {
+ String msg = "Unable to parse shape given formats" +
+ " \"lat,lon\", \"x y\" or as " + shapeReader.getFormatName() + " because " + e;
+ if (!msg.contains(str)) {
+ msg += " input: " + str;
+ }
+ throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, msg, e);
}
- return shape;
}
/**
@@ -259,11 +259,7 @@ public abstract class AbstractSpatialFieldType<T extends SpatialStrategy> extend
* The format can be selected using the initParam <code>format={WKT|GeoJSON}</code>
*/
public String shapeToString(Shape shape) {
- if(shapeWriter!=null) {
- return shapeWriter.toString(shape);
- }
- // This will only happen if the context does not have any writers
- throw new SolrException(ErrorCode.SERVER_ERROR, "ShapeWriter not configured");
+ return shapeWriter.toString(shape);
}
/** Called from {@link #getStrategy(String)} upon first use by fieldName. } */
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/fb37b3eb/solr/core/src/java/org/apache/solr/schema/RptWithGeometrySpatialField.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/schema/RptWithGeometrySpatialField.java b/solr/core/src/java/org/apache/solr/schema/RptWithGeometrySpatialField.java
index b633174..ca2771c 100644
--- a/solr/core/src/java/org/apache/solr/schema/RptWithGeometrySpatialField.java
+++ b/solr/core/src/java/org/apache/solr/schema/RptWithGeometrySpatialField.java
@@ -18,11 +18,9 @@ package org.apache.solr.schema;
import java.io.IOException;
import java.lang.ref.WeakReference;
+import java.util.HashMap;
import java.util.Map;
-import org.locationtech.spatial4j.context.SpatialContext;
-import org.locationtech.spatial4j.shape.Shape;
-import org.locationtech.spatial4j.shape.jts.JtsGeometry;
import org.apache.lucene.analysis.Analyzer;
import org.apache.lucene.index.LeafReaderContext;
import org.apache.lucene.queries.function.FunctionValues;
@@ -36,6 +34,9 @@ import org.apache.solr.common.SolrException;
import org.apache.solr.core.SolrCore;
import org.apache.solr.request.SolrRequestInfo;
import org.apache.solr.search.SolrCache;
+import org.locationtech.spatial4j.context.SpatialContext;
+import org.locationtech.spatial4j.shape.Shape;
+import org.locationtech.spatial4j.shape.jts.JtsGeometry;
/** A Solr Spatial FieldType based on {@link CompositeSpatialStrategy}.
* @lucene.experimental */
@@ -48,7 +49,8 @@ public class RptWithGeometrySpatialField extends AbstractSpatialFieldType<Compos
@Override
protected void init(IndexSchema schema, Map<String, String> args) {
- // Do NOT call super.init(); instead we delegate to an RPT field. Admittedly this is error prone.
+ Map<String, String> origArgs = new HashMap<>(args); // clone so we can feed it to an aggregated field type
+ super.init(schema, origArgs);
//TODO Move this check to a call from AbstractSpatialFieldType.createFields() so the type can declare
// if it supports multi-valued or not. It's insufficient here; we can't see if you set multiValued on the field.
@@ -65,6 +67,7 @@ public class RptWithGeometrySpatialField extends AbstractSpatialFieldType<Compos
rptFieldType.setTypeName(getTypeName());
rptFieldType.properties = properties;
rptFieldType.init(schema, args);
+
rptFieldType.argsParser = argsParser = newSpatialArgsParser();
this.ctx = rptFieldType.ctx;
this.distanceUnits = rptFieldType.distanceUnits;
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/fb37b3eb/solr/core/src/test/org/apache/solr/schema/SpatialRPTFieldTypeTest.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/schema/SpatialRPTFieldTypeTest.java b/solr/core/src/test/org/apache/solr/schema/SpatialRPTFieldTypeTest.java
index f341832..a2afa2d 100644
--- a/solr/core/src/test/org/apache/solr/schema/SpatialRPTFieldTypeTest.java
+++ b/solr/core/src/test/org/apache/solr/schema/SpatialRPTFieldTypeTest.java
@@ -21,6 +21,7 @@ import java.util.HashMap;
import java.util.Map;
import org.apache.commons.io.FileUtils;
+import org.apache.solr.common.SolrException;
import org.apache.solr.core.AbstractBadConfigTestBase;
import org.junit.After;
import org.junit.Before;
@@ -203,8 +204,8 @@ public class SpatialRPTFieldTypeTest extends AbstractBadConfigTestBase {
}
public void testShapeToFromStringWKT() throws Exception {
- // Check WKT
- setupRPTField("miles", "true", "WKT");
+ setupRPTField("miles", "true", "WKT", random().nextBoolean()
+ ? new SpatialRecursivePrefixTreeFieldType() : new RptWithGeometrySpatialField());
AbstractSpatialFieldType ftype = (AbstractSpatialFieldType)
h.getCore().getLatestSchema().getField("geo").getType();
@@ -214,11 +215,20 @@ public class SpatialRPTFieldTypeTest extends AbstractBadConfigTestBase {
String out = ftype.shapeToString(shape);
assertEquals(wkt, out);
+
+ //assert fails GeoJSON
+ try {
+ ftype.parseShape("{\"type\":\"Point\",\"coordinates\":[1,2]}");
+ fail("Should not parse GeoJSON if told format is WKT");
+ } catch (SolrException e) {// expected
+ System.out.println(e);
+ }
+
}
public void testShapeToFromStringGeoJSON() throws Exception {
- // Check WKT
- setupRPTField("miles", "true", "GeoJSON");
+ setupRPTField("miles", "true", "GeoJSON", random().nextBoolean()
+ ? new SpatialRecursivePrefixTreeFieldType() : new RptWithGeometrySpatialField());
AbstractSpatialFieldType ftype = (AbstractSpatialFieldType)
h.getCore().getLatestSchema().getField("geo").getType();
@@ -230,7 +240,7 @@ public class SpatialRPTFieldTypeTest extends AbstractBadConfigTestBase {
assertEquals(json, out);
}
- private void setupRPTField(String distanceUnits, String geo, String format) throws Exception {
+ private void setupRPTField(String distanceUnits, String geo, String format, FieldType fieldType) throws Exception {
deleteCore();
File managedSchemaFile = new File(tmpConfDir, "managed-schema");
Files.delete(managedSchemaFile.toPath()); // Delete managed-schema so it won't block parsing a new schema
@@ -243,7 +253,9 @@ public class SpatialRPTFieldTypeTest extends AbstractBadConfigTestBase {
IndexSchema oldSchema = h.getCore().getLatestSchema();
- SpatialRecursivePrefixTreeFieldType rptFieldType = new SpatialRecursivePrefixTreeFieldType();
+ if (fieldType == null) {
+ fieldType = new SpatialRecursivePrefixTreeFieldType();
+ }
Map<String, String> rptMap = new HashMap<String,String>();
if(distanceUnits!=null)
rptMap.put("distanceUnits", distanceUnits);
@@ -252,9 +264,9 @@ public class SpatialRPTFieldTypeTest extends AbstractBadConfigTestBase {
if(format!=null) {
rptMap.put("format", format);
}
- rptFieldType.init(oldSchema, rptMap);
- rptFieldType.setTypeName("location_rpt");
- SchemaField newField = new SchemaField("geo", rptFieldType, SchemaField.STORED | SchemaField.INDEXED, null);
+ fieldType.init(oldSchema, rptMap);
+ fieldType.setTypeName("location_rpt");
+ SchemaField newField = new SchemaField("geo", fieldType, SchemaField.STORED | SchemaField.INDEXED, null);
IndexSchema newSchema = oldSchema.addField(newField);
h.getCore().setLatestSchema(newSchema);
@@ -263,6 +275,6 @@ public class SpatialRPTFieldTypeTest extends AbstractBadConfigTestBase {
}
private void setupRPTField(String distanceUnits, String geo) throws Exception {
- setupRPTField(distanceUnits, geo, null);
+ setupRPTField(distanceUnits, geo, null, null);
}
}