From fb37b3eb8c4130c8b5f53e1741e9585743b26e4d Mon Sep 17 00:00:00 2001 From: David Smiley Date: Tue, 7 Jun 2016 16:48:34 -0400 Subject: [PATCH] SOLR-8859: Fix AbstractSpatialFieldType to not cycle through all Spatial4j provided formats. And Fix RptWithGeometrySpatialField to be less brittle on init() --- 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(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 56c111b8286..bed02f25c0a 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. 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 3130004593c..12fcea35f28 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 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"; } - else { - // Otherwise, pick the first supported reader/writer - shapeWriter = fmts.getWriters().get(0); - shapeReader = fmts.getReaders().get(0); + 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); + } + argsParser = newSpatialArgsParser(); } @@ -234,23 +228,29 @@ public abstract class AbstractSpatialFieldType 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 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) { - // Try all supported formats - shape = ctx.getFormats().read(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); } - - if(shape==null) { - throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Unable to parse shape from: "+str); - } - return shape; } /** @@ -259,11 +259,7 @@ public abstract class AbstractSpatialFieldType extend * The format can be selected using the initParam format={WKT|GeoJSON} */ 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. } */ 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 b633174f910..ca2771c3482 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 args) { - // Do NOT call super.init(); instead we delegate to an RPT field. Admittedly this is error prone. + Map 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 rptMap = new HashMap(); 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); } }