From 9c1d89fe1bedc9c1a0c8bf462d707e25d95f5f06 Mon Sep 17 00:00:00 2001 From: James Dyer Date: Tue, 27 Nov 2012 16:12:26 +0000 Subject: [PATCH] SOLR-2141 / SOLR-4047 / SOLR-3842 - fix problems with VariableResolver, better test coverage git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1414242 13f79535-47bb-0310-9956-ffa450edef68 --- .../dataimport/DateFormatEvaluator.java | 34 ++--- .../solr/handler/dataimport/DocBuilder.java | 14 +- .../handler/dataimport/VariableResolver.java | 11 +- .../dataimport/TestBuiltInEvaluators.java | 2 +- .../TestVariableResolverEndToEnd.java | 137 ++++++++++++++++++ 5 files changed, 172 insertions(+), 26 deletions(-) create mode 100644 solr/contrib/dataimporthandler/src/test/org/apache/solr/handler/dataimport/TestVariableResolverEndToEnd.java diff --git a/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/DateFormatEvaluator.java b/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/DateFormatEvaluator.java index 821c9d00919..5ec2903161a 100644 --- a/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/DateFormatEvaluator.java +++ b/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/DateFormatEvaluator.java @@ -67,6 +67,15 @@ public class DateFormatEvaluator extends Evaluator { availableLocales.put(locale.toString(), locale); } } + private SimpleDateFormat getDateFormat(String pattern, Locale locale) { + DateFormatCacheKey dfck = new DateFormatCacheKey(locale, pattern); + SimpleDateFormat sdf = cache.get(dfck); + if(sdf == null) { + sdf = new SimpleDateFormat(pattern, locale); + cache.put(dfck, sdf); + } + return sdf; + } @Override @@ -81,15 +90,13 @@ public class DateFormatEvaluator extends Evaluator { VariableWrapper wrapper = (VariableWrapper) format; o = wrapper.resolve(); format = o.toString(); - } + } Locale locale = Locale.ROOT; if(l.size()==3) { Object localeObj = l.get(2); String localeStr = null; if (localeObj instanceof VariableWrapper) { - VariableWrapper wrapper = (VariableWrapper) localeObj; - o = wrapper.resolve(); - localeStr = o.toString(); + localeStr = ((VariableWrapper) localeObj).resolve().toString(); } else { localeStr = localeObj.toString(); } @@ -97,14 +104,9 @@ public class DateFormatEvaluator extends Evaluator { if(locale==null) { throw new DataImportHandlerException(SEVERE, "Unsupported locale: " + localeStr); } - } + } String dateFmt = format.toString(); - DateFormatCacheKey dfck = new DateFormatCacheKey(locale, dateFmt); - SimpleDateFormat sdf = cache.get(dfck); - if(sdf==null) { - sdf = new SimpleDateFormat(dateFmt, locale); - cache.put(dfck, sdf); - } + SimpleDateFormat fmt = getDateFormat(dateFmt, locale); Date date = null; if (o instanceof VariableWrapper) { VariableWrapper variableWrapper = (VariableWrapper) o; @@ -114,13 +116,7 @@ public class DateFormatEvaluator extends Evaluator { } else { String s = variableval.toString(); try { - dfck = new DateFormatCacheKey(locale, DEFAULT_DATE_FORMAT); - sdf = cache.get(dfck); - if(sdf==null) { - sdf = new SimpleDateFormat(dfck.dateFormat, dfck.locale); - cache.put(dfck, sdf); - } - date = new SimpleDateFormat(DEFAULT_DATE_FORMAT, locale).parse(s); + date = getDateFormat(DEFAULT_DATE_FORMAT, locale).parse(s); } catch (ParseException exp) { wrapAndThrow(SEVERE, exp, "Invalid expression for date"); } @@ -134,7 +130,7 @@ public class DateFormatEvaluator extends Evaluator { wrapAndThrow(SEVERE, e, "Invalid expression for date"); } } - return sdf.format(date); + return fmt.format(date); } static DateMathParser getDateMathParser(Locale l) { return new DateMathParser(TimeZone.getDefault(), l) { diff --git a/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/DocBuilder.java b/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/DocBuilder.java index e3bf369ef4e..2513ba99ac3 100644 --- a/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/DocBuilder.java +++ b/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/DocBuilder.java @@ -68,7 +68,6 @@ public class DocBuilder { Map session = new HashMap(); static final ThreadLocal INSTANCE = new ThreadLocal(); - //private Map functionsNamespace; private Map persistedProperties; private DIHProperties propWriter; @@ -640,11 +639,20 @@ public class DocBuilder { if (field != null) { for (EntityField f : field) { String name = f.getName(); + boolean multiValued = f.isMultiValued(); + boolean toWrite = f.isToWrite(); if(f.isDynamicName()){ name = vr.replaceTokens(name); + SchemaField schemaField = dataImporter.getSchemaField(name); + if(schemaField == null) { + toWrite = false; + } else { + multiValued = schemaField.multiValued(); + toWrite = true; + } } - if (f.isToWrite()) { - addFieldToDoc(entry.getValue(), name, f.getBoost(), f.isMultiValued(), doc); + if (toWrite) { + addFieldToDoc(entry.getValue(), name, f.getBoost(), multiValued, doc); } } } diff --git a/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/VariableResolver.java b/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/VariableResolver.java index 8bbd1b17032..7a324532536 100644 --- a/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/VariableResolver.java +++ b/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/VariableResolver.java @@ -63,6 +63,7 @@ public class VariableResolver { } public static final String FUNCTIONS_NAMESPACE = "dataimporter.functions."; + public static final String FUNCTIONS_NAMESPACE_SHORT = "dih.functions."; public VariableResolver() { rootNamespace = new HashMap(); @@ -95,7 +96,11 @@ public class VariableResolver { r = currentLevel.get(nameParts[nameParts.length - 1]); if (r == null && name.startsWith(FUNCTIONS_NAMESPACE) && name.length() > FUNCTIONS_NAMESPACE.length()) { - return resolveEvaluator(name); + return resolveEvaluator(FUNCTIONS_NAMESPACE, name); + } + if (r == null && name.startsWith(FUNCTIONS_NAMESPACE_SHORT) + && name.length() > FUNCTIONS_NAMESPACE_SHORT.length()) { + return resolveEvaluator(FUNCTIONS_NAMESPACE_SHORT, name); } if (r == null) { r = System.getProperty(name); @@ -104,12 +109,12 @@ public class VariableResolver { return r == null ? "" : r; } - private Object resolveEvaluator(String name) { + private Object resolveEvaluator(String namespace, String name) { if (evaluators == null) { return ""; } Matcher m = EVALUATOR_FORMAT_PATTERN.matcher(name - .substring(FUNCTIONS_NAMESPACE.length())); + .substring(namespace.length())); if (m.find()) { String fname = m.group(1); Evaluator evaluator = evaluators.get(fname); diff --git a/solr/contrib/dataimporthandler/src/test/org/apache/solr/handler/dataimport/TestBuiltInEvaluators.java b/solr/contrib/dataimporthandler/src/test/org/apache/solr/handler/dataimport/TestBuiltInEvaluators.java index 03b80490aa3..27fcf8b9f3e 100644 --- a/solr/contrib/dataimporthandler/src/test/org/apache/solr/handler/dataimport/TestBuiltInEvaluators.java +++ b/solr/contrib/dataimporthandler/src/test/org/apache/solr/handler/dataimport/TestBuiltInEvaluators.java @@ -24,7 +24,7 @@ import java.text.SimpleDateFormat; import java.util.*; /** - *

Test for EvaluatorBag

+ *

Test for Evaluators

* * * @since solr 1.3 diff --git a/solr/contrib/dataimporthandler/src/test/org/apache/solr/handler/dataimport/TestVariableResolverEndToEnd.java b/solr/contrib/dataimporthandler/src/test/org/apache/solr/handler/dataimport/TestVariableResolverEndToEnd.java new file mode 100644 index 00000000000..128805351fb --- /dev/null +++ b/solr/contrib/dataimporthandler/src/test/org/apache/solr/handler/dataimport/TestVariableResolverEndToEnd.java @@ -0,0 +1,137 @@ +package org.apache.solr.handler.dataimport; + +import java.sql.Connection; +import java.sql.Statement; +import java.util.Locale; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import junit.framework.Assert; + +import org.apache.solr.request.SolrQueryRequest; +import org.junit.Test; + +/* + * 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. + */ + +public class TestVariableResolverEndToEnd extends AbstractDIHJdbcTestCase { + + @Test + public void test() throws Exception { + h.query("/dataimport", generateRequest()); + SolrQueryRequest req = null; + try { + req = req("q", "*:*", "wt", "json", "indent", "true"); + String response = h.query(req); + log.debug(response); + response = response.replaceAll("\\s",""); + Assert.assertTrue(response.contains("\"numFound\":1")); + Pattern p = Pattern.compile("[\"]second1_s[\"][:][\"](.*?)[\"]"); + Matcher m = p.matcher(response); + Assert.assertTrue(m.find()); + String yearStr = m.group(1); + Assert.assertTrue(response.contains("\"second1_s\":\"" + yearStr + "\"")); + Assert.assertTrue(response.contains("\"second2_s\":\"" + yearStr + "\"")); + Assert.assertTrue(response.contains("\"second3_s\":\"" + yearStr + "\"")); + Assert.assertTrue(response.contains("\"PORK_s\":\"GRILL\"")); + Assert.assertTrue(response.contains("\"FISH_s\":\"FRY\"")); + Assert.assertTrue(response.contains("\"BEEF_CUTS_mult_s\":[\"ROUND\",\"SIRLOIN\"]")); + } catch(Exception e) { + throw e; + } finally { + req.close(); + } + } + + @Override + protected String generateConfig() { + String thirdLocaleParam = random().nextBoolean() ? "" : (", '" + Locale.getDefault() + "'"); + StringBuilder sb = new StringBuilder(); + sb.append(" \n"); + sb.append(" \n"); + sb.append(" \n"); + sb.append("\n"); + sb.append(" \n"); + sb.append(" \n"); + sb.append(" \n"); + sb.append(" \n"); + sb.append(" \n"); + sb.append(" \n"); + sb.append(" \n"); + sb.append(" \n"); + sb.append(" \n"); + sb.append("\n"); + sb.append(" \n"); + sb.append(" \n"); + String config = sb.toString(); + log.debug(config); + return config; + } + @Override + protected void populateData(Connection conn) throws Exception { + Statement s = null; + try { + s = conn.createStatement(); + s.executeUpdate("create table dual(dual char(1) not null)"); + s.executeUpdate("insert into dual values('Y')"); + conn.commit(); + } catch (Exception e) { + throw e; + } finally { + try { + s.close(); + } catch (Exception ex) {} + try { + conn.close(); + } catch (Exception ex) {} + } + } + @Override + protected Database setAllowedDatabases() { + return Database.HSQLDB; + } +}