diff --git a/CHANGES.txt b/CHANGES.txt index 1e23d0d6073..373e3337dcb 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -151,6 +151,11 @@ Changes in runtime behavior a unique id if it is declared in the schema and allowDups=false. (ryan via klaas) +17. SOLR-183: Exceptions with error code 400 are raised when + numeric argument parsing fails. RequiredSolrParams class added + to facilitate checking for parameters that must be present. + (Ryan McKinley, J.J. Larrea via yonik) + Optimizations 1. SOLR-114: HashDocSet specific implementations of union() and andNot() for a 20x performance improvement for those set operations, and a new diff --git a/src/java/org/apache/solr/request/RequiredSolrParams.java b/src/java/org/apache/solr/request/RequiredSolrParams.java new file mode 100755 index 00000000000..e18d09d40e1 --- /dev/null +++ b/src/java/org/apache/solr/request/RequiredSolrParams.java @@ -0,0 +1,109 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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.apache.solr.request; + +import org.apache.solr.core.SolrException; + +import java.util.Iterator; + +/** + * This is a simple wrapper to SolrParams that will throw a 400 + * exception if you ask for a parameter that does not exist. Fields + * specified with + * + * In short, any value you for from a RequiredSolrParams + * will return a valid non-null value or throw a 400 exception. + * (If you pass in null as the default value, you can + * get a null return value) + * + * @author jjl + * @version $Id$ + * @since solr 1.2 + */ +public class RequiredSolrParams extends SolrParams { + protected final SolrParams params; + + public RequiredSolrParams(SolrParams params) { + this.params = params; + } + + /** get the param from params, fail if not found **/ + @Override + public String get(String param) { + String val = params.get(param); + if( val == null ) { + throw new SolrException( 400, "Missing required parameter: "+param ); + } + return val; + } + + @Override + public String[] getParams(String param) { + String[] vals = params.getParams(param); + if( vals == null || vals.length == 0 ) { + throw new SolrException( 400, "Missing required parameter: "+param ); + } + return vals; + } + + /** returns an Iterator over the parameter names */ + @Override + public Iterator getParameterNamesIterator() { + return params.getParameterNamesIterator(); + } + + @Override + public String toString() { + return "{required("+params+")}"; + } + + //---------------------------------------------------------- + // Functions with a default value - pass directly to the + // wrapped SolrParams (they won't return null - unless its the default) + //---------------------------------------------------------- + + @Override + public String get(String param, String def) { + return params.get(param, def); + } + + @Override + public int getInt(String param, int def) { + return params.getInt(param, def); + } + + @Override + public float getFloat(String param, float def) { + return params.getFloat(param, def); + } + + @Override + public boolean getBool(String param, boolean def) { + return params.getBool(param, def); + } + + @Override + public int getFieldInt(String field, String param, int def) { + return params.getFieldInt(field, param, def); + } + + @Override + public boolean getFieldBool(String field, String param, boolean def) { + return params.getFieldBool(field, param, def); + } +} diff --git a/src/java/org/apache/solr/request/SolrParams.java b/src/java/org/apache/solr/request/SolrParams.java index dd2d5e1718f..37eecb3e35b 100644 --- a/src/java/org/apache/solr/request/SolrParams.java +++ b/src/java/org/apache/solr/request/SolrParams.java @@ -17,18 +17,13 @@ package org.apache.solr.request; -import org.apache.solr.util.NamedList; -import org.apache.solr.util.StrUtils; -import org.apache.solr.util.SimpleOrderedMap; - -import javax.servlet.ServletRequest; - -import java.util.ArrayList; import java.util.HashMap; import java.util.Iterator; import java.util.Map; -import java.util.Set; -import java.io.IOException; + +import org.apache.solr.core.SolrException; +import org.apache.solr.util.NamedList; +import org.apache.solr.util.SimpleOrderedMap; /** SolrParams hold request parameters. * @@ -142,6 +137,13 @@ public abstract class SolrParams { return val==null ? def : val; } + /** returns a RequiredSolrParams wrapping this */ + public RequiredSolrParams required() + { + // TODO? should we want to stash a reference? + return new RequiredSolrParams(this); + } + protected String fpname(String field, String param) { return "f."+field+'.'+param; } @@ -183,47 +185,85 @@ public abstract class SolrParams { /** Returns the Integer value of the param, or null if not set */ public Integer getInt(String param) { String val = get(param); - return val==null ? null : Integer.parseInt(val); + try { + return val==null ? null : Integer.valueOf(val); + } + catch( Exception ex ) { + throw new SolrException( 400, ex.getMessage(), ex ); + } } /** Returns the int value of the param, or def if not set */ public int getInt(String param, int def) { String val = get(param); - return val==null ? def : Integer.parseInt(val); + try { + return val==null ? def : Integer.parseInt(val); + } + catch( Exception ex ) { + throw new SolrException( 400, ex.getMessage(), ex ); + } } - + /** Returns the int value of the field param, or the value for param, or def if neither is set. */ public Integer getFieldInt(String field, String param) { String val = getFieldParam(field, param); - return val==null ? null : Integer.parseInt(val); + try { + return val==null ? null : Integer.valueOf(val); + } + catch( Exception ex ) { + throw new SolrException( 400, ex.getMessage(), ex ); + } } /** Returns the int value of the field param, or the value for param, or def if neither is set. */ public int getFieldInt(String field, String param, int def) { String val = getFieldParam(field, param); - return val==null ? def : Integer.parseInt(val); + try { + return val==null ? def : Integer.parseInt(val); + } + catch( Exception ex ) { + throw new SolrException( 400, ex.getMessage(), ex ); + } } /** Returns the Float value of the param, or null if not set */ public Float getFloat(String param) { String val = get(param); - return val==null ? null : Float.parseFloat(val); + try { + return val==null ? null : Float.valueOf(val); + } + catch( Exception ex ) { + throw new SolrException( 400, ex.getMessage(), ex ); + } } /** Returns the float value of the param, or def if not set */ public float getFloat(String param, float def) { String val = get(param); - return val==null ? def : Float.parseFloat(val); + try { + return val==null ? def : Float.parseFloat(val); + } + catch( Exception ex ) { + throw new SolrException( 400, ex.getMessage(), ex ); + } } /** how to transform a String into a boolean... more flexible than * Boolean.parseBoolean() to enable easier integration with html forms. */ protected boolean parseBool(String s) { - return s.startsWith("true") || s.startsWith("on") || s.startsWith("yes"); + if( s != null ) { + if( s.startsWith("true") || s.startsWith("on") || s.startsWith("yes") ) { + return true; + } + if( s.startsWith("false") || s.startsWith("off") || s.equals("no") ) { + return false; + } + } + throw new SolrException( 400, "invalid boolean value: "+s ); } /** Create a Map from a NamedList given no keys are repeated */ @@ -258,8 +298,8 @@ public abstract class SolrParams { } /** Convert this to a NamedList */ - public NamedList toNamedList() { - final SimpleOrderedMap result = new SimpleOrderedMap(); + public NamedList toNamedList() { + final SimpleOrderedMap result = new SimpleOrderedMap(); for(Iterator it=getParameterNamesIterator(); it.hasNext(); ) { final String name = it.next(); diff --git a/src/test/org/apache/solr/util/SolrParamTest.java b/src/test/org/apache/solr/util/SolrParamTest.java new file mode 100755 index 00000000000..029d91e7809 --- /dev/null +++ b/src/test/org/apache/solr/util/SolrParamTest.java @@ -0,0 +1,183 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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.apache.solr.util; + +import junit.framework.TestCase; + +import java.util.HashMap; +import java.util.Map; + +import org.apache.solr.core.SolrException; +import org.apache.solr.request.SolrParams; +import org.apache.solr.request.MapSolrParams; +import org.apache.solr.request.DefaultSolrParams; + +/** + * @author ryan + */ +public class SolrParamTest extends TestCase +{ + public void testGetParams() { + Map pmap = new HashMap(); + pmap.put( "str" , "string" ); + pmap.put( "bool" , "true" ); + pmap.put( "true-0" , "true" ); + pmap.put( "true-1" , "yes" ); + pmap.put( "true-2" , "on" ); + pmap.put( "false-0" , "false" ); + pmap.put( "false-1" , "off" ); + pmap.put( "false-2" , "no" ); + pmap.put( "int" , "100" ); + pmap.put( "float" , "10.6" ); + pmap.put( "f.fl.str" , "string" ); + pmap.put( "f.fl.bool" , "true" ); + pmap.put( "f.fl.int" , "100" ); + pmap.put( "f.fl.float" , "10.6" ); + pmap.put( "f.bad.bool" , "notbool" ); + pmap.put( "f.bad.int" , "notint" ); + pmap.put( "f.bad.float", "notfloat" ); + final SolrParams params = new MapSolrParams( pmap ); + + // Test the string values we put in directly + assertEquals( "string" , params.get( "str" ) ); + assertEquals( "true" , params.get( "bool" ) ); + assertEquals( "100" , params.get( "int" ) ); + assertEquals( "10.6" , params.get( "float" ) ); + assertEquals( "string" , params.get( "f.fl.str" ) ); + assertEquals( "true" , params.get( "f.fl.bool" ) ); + assertEquals( "100" , params.get( "f.fl.int" ) ); + assertEquals( "10.6" , params.get( "f.fl.float" ) ); + assertEquals( "notbool" , params.get( "f.bad.bool" ) ); + assertEquals( "notint" , params.get( "f.bad.int" ) ); + assertEquals( "notfloat" , params.get( "f.bad.float" ) ); + + final String pstr = "string"; + final Boolean pbool = Boolean.TRUE; + final Integer pint = new Integer( 100 ); + final Float pfloat = new Float( 10.6f ); + + // Make sure they parse ok + assertEquals( pstr , params.get( "str" ) ); + assertEquals( pbool , params.getBool( "bool" ) ); + assertEquals( pint , params.getInt( "int" ) ); + assertEquals( pfloat , params.getFloat( "float" ) ); + assertEquals( pbool , params.getBool( "f.fl.bool" ) ); + assertEquals( pint , params.getInt( "f.fl.int" ) ); + assertEquals( pfloat , params.getFloat( "f.fl.float" ) ); + assertEquals( pstr , params.getFieldParam( "fl", "str" ) ); + assertEquals( pbool , params.getFieldBool( "fl", "bool" ) ); + assertEquals( pint , params.getFieldInt( "fl", "int" ) ); + + // Test field defaulting (fall through to non-field-specific value) + assertEquals( pint , params.getFieldInt( "fff", "int" ) ); + + // test boolean parsing + for( int i=0; i<3; i++ ) { + // Must use Boolean rather than boolean reference value to prevent + // auto-unboxing ambiguity + assertEquals( Boolean.TRUE, params.getBool( "true-"+i ) ); + assertEquals( Boolean.FALSE, params.getBool( "false-"+i ) ); + } + + // Malformed params: These should throw a 400 + assertEquals( 400, getReturnCode( new Runnable() { public void run() { params.getInt( "f.bad.int" ); } } ) ); + assertEquals( 400, getReturnCode( new Runnable() { public void run() { params.getBool( "f.bad.bool" ); } } ) ); + assertEquals( 400, getReturnCode( new Runnable() { public void run() { params.getFloat( "f.bad.float" ); } } ) ); + + // Ask for params that arent there + assertNull( params.get( "asagdsaga" ) ); + assertNull( params.getBool( "asagdsaga" ) ); + assertNull( params.getInt( "asagdsaga" ) ); + assertNull( params.getFloat( "asagdsaga" ) ); + + // Get things with defaults + assertEquals( pstr , params.get( "xxx", pstr ) ); + assertEquals( pbool.booleanValue() , params.getBool( "xxx", pbool ) ); + assertEquals( pint.intValue() , params.getInt( "xxx", pint ) ); + assertEquals( pfloat.floatValue() , params.getFloat( "xxx", pfloat ) ); + assertEquals( pbool.booleanValue() , params.getFieldBool( "xxx", "bool", pbool ) ); + assertEquals( pint.intValue() , params.getFieldInt( "xxx", "int", pint ) ); + + // Required params testing uses decorator + final SolrParams required = params.required(); + + // Required params which are present should test same as above + assertEquals( pstr , required.get( "str" ) ); + assertEquals( pbool , required.getBool( "bool" ) ); + assertEquals( pint , required.getInt( "int" ) ); + assertEquals( pfloat , required.getFloat( "float" ) ); + + // field value present + assertEquals( pbool , required.getFieldBool( "fl", "bool" ) ); + // field defaulting (fall through to non-field-specific value) + //assertEquals( pint , required.getFieldInt( "fff", "int" ) ); + + // Required params which are missing: These should throw a 400 + assertEquals( 400, getReturnCode( new Runnable() { public void run() { required.get( "aaaa" ); } } ) ); + assertEquals( 400, getReturnCode( new Runnable() { public void run() { required.getInt( "f.bad.int" ); } } ) ); + assertEquals( 400, getReturnCode( new Runnable() { public void run() { required.getBool( "f.bad.bool" ); } } ) ); + assertEquals( 400, getReturnCode( new Runnable() { public void run() { required.getFloat( "f.bad.float" ); } } ) ); + assertEquals( 400, getReturnCode( new Runnable() { public void run() { required.getInt( "aaa" ); } } ) ); + assertEquals( 400, getReturnCode( new Runnable() { public void run() { required.getBool( "aaa" ); } } ) ); + assertEquals( 400, getReturnCode( new Runnable() { public void run() { required.getFloat( "aaa" ); } } ) ); + assertEquals( 400, getReturnCode( new Runnable() { public void run() { params.getFieldBool( "bad", "bool" ); } } ) ); + assertEquals( 400, getReturnCode( new Runnable() { public void run() { params.getFieldInt( "bad", "int" ); } } ) ); + + // Fields with default use their parent value: + assertEquals( + params.get( "aaaa", "str" ), + required.get( "aaaa", "str" ) ); + assertEquals( + params.getInt( "f.bad.nnnn", pint ), + required.getInt( "f.bad.nnnn", pint ) ); + + // Check default SolrParams + Map dmap = new HashMap(); + // these are not defined in params + dmap.put( "dstr" , "default" ); + dmap.put( "dint" , "123" ); + // these are defined in params + dmap.put( "int" , "456" ); + SolrParams defaults = new DefaultSolrParams( params, new MapSolrParams( dmap ) ); + + // in params, not in default + assertEquals( pstr , defaults.get( "str" ) ); + // in default, not in params + assertEquals( "default" , defaults.get( "dstr" ) ); + assertEquals( new Integer(123) , defaults.getInt( "dint" ) ); + // in params, overriding defaults + assertEquals( pint , defaults.getInt( "int" ) ); + // in neither params nor defaults + assertNull( defaults.get( "asagdsaga" ) ); + } + + public static int getReturnCode( Runnable runnable ) + { + try { + runnable.run(); + } + catch( SolrException sx ) { + return sx.code(); + } + catch( Exception ex ) { + ex.printStackTrace(); + return 500; + } + return 200; + } +}