[teiid-commits] teiid SVN: r2616 - in branches/7.1.x/engine/src: main/java/org/teiid/query/metadata and 2 other directories.
teiid-commits at lists.jboss.org
teiid-commits at lists.jboss.org
Mon Sep 27 16:39:44 EDT 2010
Author: shawkins
Date: 2010-09-27 16:39:44 -0400 (Mon, 27 Sep 2010)
New Revision: 2616
Modified:
branches/7.1.x/engine/src/main/java/org/teiid/query/function/FunctionTree.java
branches/7.1.x/engine/src/main/java/org/teiid/query/function/SystemFunctionManager.java
branches/7.1.x/engine/src/main/java/org/teiid/query/metadata/TransformationMetadata.java
branches/7.1.x/engine/src/main/resources/org/teiid/query/i18n.properties
branches/7.1.x/engine/src/test/java/org/teiid/query/function/TestFunctionDescriptorImpl.java
branches/7.1.x/engine/src/test/java/org/teiid/query/function/TestFunctionTree.java
Log:
TEIID-1279 clarifying errors when loading udfs
Modified: branches/7.1.x/engine/src/main/java/org/teiid/query/function/FunctionTree.java
===================================================================
--- branches/7.1.x/engine/src/main/java/org/teiid/query/function/FunctionTree.java 2010-09-27 20:03:53 UTC (rev 2615)
+++ branches/7.1.x/engine/src/main/java/org/teiid/query/function/FunctionTree.java 2010-09-27 20:39:44 UTC (rev 2616)
@@ -72,46 +72,27 @@
* Function lookup and invocation use: Function name (uppercase) to Map (recursive tree)
*/
private Map treeRoot = new HashMap();
+ private boolean validateClass;
/**
* Construct a new tree with the given source of function metadata.
* @param source The metadata source
*/
public FunctionTree(FunctionMetadataSource source) {
- // Load data structures
- addSource(source);
+ this(source, false);
}
-
+
/**
- * Construct a new tree with the given collection of sources.
- * @param sources The collection of function metadata sources ({@link org.teiid.query.function.FunctionMetadataSource})
+ * Construct a new tree with the given source of function metadata.
+ * @param source The metadata source
*/
- FunctionTree(Collection sources) {
+ public FunctionTree(FunctionMetadataSource source, boolean validateClass) {
// Load data structures
- addSources(sources);
+ this.validateClass = validateClass;
+ addSource(source);
}
/**
- * Add a collection of functions to the data structures.
- * @param sources The function metadata sources ({@link org.teiid.query.function.FunctionMetadataSource})
- */
- private void addSources(Collection sources) {
- if(sources == null) {
- return;
- }
-
- Iterator sourceIter = sources.iterator();
- while(sourceIter.hasNext()) {
- Object sourceObj = sourceIter.next();
- if(sourceObj instanceof FunctionMetadataSource) {
- addSource((FunctionMetadataSource) sourceObj);
- } else {
- Assertion.failed(QueryPlugin.Util.getString("ERR.015.001.0044", sourceObj.getClass().getName())); //$NON-NLS-1$
- }
- }
- }
-
- /**
* Add all functions from a metadata source to the data structures.
* @param source The source of the functions
*/
@@ -294,7 +275,7 @@
// Defect 20007 - Ignore the invocation method if pushdown is not required.
if (method.getPushdown() == FunctionMethod.CAN_PUSHDOWN || method.getPushdown() == FunctionMethod.CANNOT_PUSHDOWN) {
try {
- Class methodClass = source.getInvocationClass(method.getInvocationClass());
+ Class<?> methodClass = source.getInvocationClass(method.getInvocationClass());
ReflectionHelper helper = new ReflectionHelper(methodClass);
try {
invocationMethod = helper.findBestMethodWithSignature(method.getInvocationMethod(), inputTypes);
@@ -304,15 +285,31 @@
requiresContext = true;
}
} catch (ClassNotFoundException e) {
- // Failed to load class, so can't load method - this will fail at invocation time.
- // We don't fail here because this situation can occur in the modeler, which does
- // not have the function jar files. The modeler never invokes, so this isn't a
- // problem.
+ if (validateClass) {
+ throw new TeiidRuntimeException(e, "ERR.015.001.0047", QueryPlugin.Util.getString("FunctionTree.no_class", method.getName(), method.getInvocationClass())); //$NON-NLS-1$ //$NON-NLS-2$
+ }
+ } catch (NoSuchMethodException e) {
+ throw new TeiidRuntimeException(e, "ERR.015.001.0047", QueryPlugin.Util.getString("FunctionTree.no_method", method, method.getInvocationClass(), method.getInvocationMethod())); //$NON-NLS-1$ //$NON-NLS-2$
} catch (Exception e) {
- throw new TeiidRuntimeException(e, "ERR.015.001.0047", QueryPlugin.Util.getString("ERR.015.001.0047", new Object[]{method.getInvocationClass(), invocationMethod, inputTypes})); //$NON-NLS-1$ //$NON-NLS-2$
+ throw new TeiidRuntimeException(e, "ERR.015.001.0047", QueryPlugin.Util.getString("ERR.015.001.0047", method, method.getInvocationClass(), method.getInvocationMethod())); //$NON-NLS-1$ //$NON-NLS-2$
}
- if(invocationMethod != null && !FunctionTree.isValidMethod(invocationMethod)) {
- throw new TeiidRuntimeException("ERR.015.001.0047", QueryPlugin.Util.getString("ERR.015.001.0047", new Object[]{method.getInvocationClass(), invocationMethod, inputTypes})); //$NON-NLS-1$ //$NON-NLS-2$
+ if (invocationMethod != null) {
+ // Check return type is non void
+ Class<?> methodReturn = invocationMethod.getReturnType();
+ if(methodReturn.equals(Void.TYPE)) {
+ throw new TeiidRuntimeException("ERR.015.001.0047", QueryPlugin.Util.getString("FunctionTree.not_void", method.getName(), invocationMethod)); //$NON-NLS-1$ //$NON-NLS-2$
+ }
+
+ // Check that method is public
+ int modifiers = invocationMethod.getModifiers();
+ if(! Modifier.isPublic(modifiers)) {
+ throw new TeiidRuntimeException("ERR.015.001.0047", QueryPlugin.Util.getString("FunctionTree.not_public", method.getName(), invocationMethod)); //$NON-NLS-1$ //$NON-NLS-2$
+ }
+
+ // Check that method is static
+ if(! Modifier.isStatic(modifiers)) {
+ throw new TeiidRuntimeException("ERR.015.001.0047", QueryPlugin.Util.getString("FunctionTree.not_static", method.getName(), invocationMethod)); //$NON-NLS-1$ //$NON-NLS-2$
+ }
}
} else {
inputTypes.add(0, CommandContext.class);
@@ -342,33 +339,6 @@
node.put(DESCRIPTOR_KEY, descriptor);
}
- /**
- * Validate a method looked up by reflection. The method should have a non-void return type
- * and be a public static method.
- * @param method Method to validate
- * @return True if valid
- */
- static boolean isValidMethod(Method method) {
- // Check return type is non void
- Class methodReturn = method.getReturnType();
- if(methodReturn.equals(Void.TYPE)) {
- return false;
- }
-
- // Check that method is public
- int modifiers = method.getModifiers();
- if(! Modifier.isPublic(modifiers)) {
- return false;
- }
-
- // Check that method is static
- if(! Modifier.isStatic(modifiers)) {
- return false;
- }
-
- return true;
- }
-
/**
* Look up a function descriptor by signature in the tree. If none is
* found, null is returned.
Modified: branches/7.1.x/engine/src/main/java/org/teiid/query/function/SystemFunctionManager.java
===================================================================
--- branches/7.1.x/engine/src/main/java/org/teiid/query/function/SystemFunctionManager.java 2010-09-27 20:03:53 UTC (rev 2615)
+++ branches/7.1.x/engine/src/main/java/org/teiid/query/function/SystemFunctionManager.java 2010-09-27 20:39:44 UTC (rev 2616)
@@ -46,7 +46,7 @@
System.err.println(QueryPlugin.Util.getString("ERR.015.001.0005", report)); //$NON-NLS-1$
}
- systemFunctionTree = new FunctionTree(systemSource);
+ systemFunctionTree = new FunctionTree(systemSource, true);
}
Modified: branches/7.1.x/engine/src/main/java/org/teiid/query/metadata/TransformationMetadata.java
===================================================================
--- branches/7.1.x/engine/src/main/java/org/teiid/query/metadata/TransformationMetadata.java 2010-09-27 20:03:53 UTC (rev 2615)
+++ branches/7.1.x/engine/src/main/java/org/teiid/query/metadata/TransformationMetadata.java 2010-09-27 20:39:44 UTC (rev 2616)
@@ -157,7 +157,7 @@
if (udfMethods == null) {
udfMethods = Collections.emptyList();
}
- this.functionLibrary = new FunctionLibrary(SystemFunctionManager.getSystemFunctions(), new FunctionTree(new UDFSource(udfMethods)));
+ this.functionLibrary = new FunctionLibrary(SystemFunctionManager.getSystemFunctions(), new FunctionTree(new UDFSource(udfMethods), true));
}
//==================================================================================
Modified: branches/7.1.x/engine/src/main/resources/org/teiid/query/i18n.properties
===================================================================
--- branches/7.1.x/engine/src/main/resources/org/teiid/query/i18n.properties 2010-09-27 20:03:53 UTC (rev 2615)
+++ branches/7.1.x/engine/src/main/resources/org/teiid/query/i18n.properties 2010-09-27 20:39:44 UTC (rev 2616)
@@ -41,7 +41,12 @@
ERR.015.001.0044 = Function metadata source is of invalid type: {0}
ERR.015.001.0045 = Function method is of invalid type: {0}
ERR.015.001.0046 = The function "{0}" will not be added because a function with the same name and signature already exists.
-ERR.015.001.0047 = Unexpected exception while loading {0}.{1} with args= {2}
+ERR.015.001.0047 = Unexpected exception while loading "{1}.{2}" for UDF "{0}"
+FunctionTree.not_void = UDF "{0}" method "{1}" must not return void.
+FunctionTree.not_public = UDF "{0}" method "{1}" must be public.
+FunctionTree.not_static = UDF "{0}" method "{1}" must be static.
+FunctionTree.no_class = Could not load UDF "{0}", since its invocation class "{1}" could not be found.
+FunctionTree.no_method = UDF "{0}" could not loaded, since no method on class "{1}" with name "{2}" has a matching type signature.
ERR.015.001.0048 = Unable to represent average value from {0} / {1}
ERR.015.001.0050 = Unable to compute aggregate function {0} on data of type {1}
ERR.015.001.0052 = {0} must be non-null.
Modified: branches/7.1.x/engine/src/test/java/org/teiid/query/function/TestFunctionDescriptorImpl.java
===================================================================
--- branches/7.1.x/engine/src/test/java/org/teiid/query/function/TestFunctionDescriptorImpl.java 2010-09-27 20:03:53 UTC (rev 2615)
+++ branches/7.1.x/engine/src/test/java/org/teiid/query/function/TestFunctionDescriptorImpl.java 2010-09-27 20:39:44 UTC (rev 2616)
@@ -24,15 +24,13 @@
import java.lang.reflect.Method;
+import junit.framework.TestCase;
+
import org.teiid.core.types.DataTypeManager;
import org.teiid.core.util.UnitTestUtil;
-import org.teiid.query.function.FunctionDescriptor;
-import org.teiid.query.function.FunctionTree;
import org.teiid.query.function.metadata.FunctionMethod;
-import junit.framework.TestCase;
-
public class TestFunctionDescriptorImpl extends TestCase {
/**
@@ -72,10 +70,6 @@
return null;
}
- // Validate method
- if(! FunctionTree.isValidMethod(method)) {
- return null;
- }
return method;
}
Modified: branches/7.1.x/engine/src/test/java/org/teiid/query/function/TestFunctionTree.java
===================================================================
--- branches/7.1.x/engine/src/test/java/org/teiid/query/function/TestFunctionTree.java 2010-09-27 20:03:53 UTC (rev 2615)
+++ branches/7.1.x/engine/src/test/java/org/teiid/query/function/TestFunctionTree.java 2010-09-27 20:39:44 UTC (rev 2616)
@@ -22,96 +22,117 @@
package org.teiid.query.function;
-import java.util.ArrayList;
+import static org.junit.Assert.*;
+
import java.util.Arrays;
import java.util.Collection;
-import java.util.Iterator;
-import java.util.List;
-import junit.framework.TestCase;
-
+import org.junit.Test;
import org.mockito.Mockito;
+import org.teiid.core.TeiidRuntimeException;
import org.teiid.core.types.DataTypeManager;
-import org.teiid.logging.LogManager;
-import org.teiid.query.function.FunctionForm;
-import org.teiid.query.function.FunctionLibrary;
-import org.teiid.query.function.FunctionMetadataSource;
-import org.teiid.query.function.FunctionTree;
-import org.teiid.query.function.SystemFunctionManager;
-import org.teiid.query.function.UDFSource;
import org.teiid.query.function.metadata.FunctionCategoryConstants;
import org.teiid.query.function.metadata.FunctionMethod;
import org.teiid.query.function.metadata.FunctionParameter;
import org.teiid.query.function.source.SystemSource;
+ at SuppressWarnings("nls")
+public class TestFunctionTree {
-public class TestFunctionTree extends TestCase {
-
- // ################################## FRAMEWORK ################################
-
- public TestFunctionTree(String name) {
- super(name);
- }
-
- // ################################## TEST HELPERS ################################
-
- // ################################## ACTUAL TESTS ################################
-
/**
* Walk through all functions by metadata and verify that we can look
* each one up by signature
*/
- public void testWalkTree() {
+ @Test public void testWalkTree() {
SystemSource source = new SystemSource();
FunctionTree ft = new FunctionTree(source);
- Collection categories = ft.getCategories();
- Iterator catIter = categories.iterator();
- while(catIter.hasNext()) {
- String category = (String) catIter.next();
- LogManager.logInfo("test", "Category: " + category); //$NON-NLS-1$ //$NON-NLS-2$
-
- Collection functions = ft.getFunctionForms(category);
- Iterator functionIter = functions.iterator();
- while(functionIter.hasNext()) {
- FunctionForm form = (FunctionForm) functionIter.next();
- LogManager.logInfo("test", "\tFunction: " + form.getDisplayString()); //$NON-NLS-1$ //$NON-NLS-2$
- }
+ Collection<String> categories = ft.getCategories();
+ for (String category : categories) {
+ Collection<FunctionForm> functions = ft.getFunctionForms(category);
+ assertTrue(functions.size() > 0);
}
}
- /**
- * Test what happens when a function is loaded that does not have a class in the
- * classpath. This *should* be ok as long as the function is not invoked.
- */
- public void testUnloadableFunction() {
- // Create dummy source
- FunctionMetadataSource dummySource = new FunctionMetadataSource() {
- public Collection getFunctionMethods() {
- // Build dummy method
- FunctionMethod method = new FunctionMethod(
- "dummy", null, "no category", "nonexistentClass", "noMethod", //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$
- new FunctionParameter[0],
- new FunctionParameter("output", DataTypeManager.DefaultDataTypes.STRING) ); //$NON-NLS-1$
+ public String z() {
+ return null;
+ }
+
+ protected static String x() {
+ return null;
+ }
+
+ public static String y() {
+ return null;
+ }
+
+ @Test public void testLoadErrors() {
+ FunctionMethod method = new FunctionMethod(
+ "dummy", null, null, FunctionMethod.CAN_PUSHDOWN, "nonexistentClass", "noMethod", //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
+ new FunctionParameter[0],
+ new FunctionParameter("output", DataTypeManager.DefaultDataTypes.STRING)); //$NON-NLS-1$
+
+ //allowed, since we're not validating the class
+ new FunctionLibrary(SystemFunctionManager.getSystemFunctions(), new FunctionTree(new UDFSource(Arrays.asList(method))));
+
+ //should fail, no class
+ try {
+ new FunctionLibrary(SystemFunctionManager.getSystemFunctions(), new FunctionTree(new UDFSource(Arrays.asList(method)), true));
+ fail();
+ } catch (TeiidRuntimeException e) {
+
+ }
+
+ method.setInvocationClass(TestFunctionTree.class.getName());
+
+ //should fail, no method
+ try {
+ new FunctionLibrary(SystemFunctionManager.getSystemFunctions(), new FunctionTree(new UDFSource(Arrays.asList(method)), true));
+ fail();
+ } catch (TeiidRuntimeException e) {
+
+ }
+
+ method.setInvocationMethod("testLoadErrors");
+
+ //should fail, not void
+ try {
+ new FunctionLibrary(SystemFunctionManager.getSystemFunctions(), new FunctionTree(new UDFSource(Arrays.asList(method)), true));
+ fail();
+ } catch (TeiidRuntimeException e) {
+
+ }
+
+ method.setInvocationMethod("x");
+
+ //should fail, not public
+ try {
+ new FunctionLibrary(SystemFunctionManager.getSystemFunctions(), new FunctionTree(new UDFSource(Arrays.asList(method)), true));
+ fail();
+ } catch (TeiidRuntimeException e) {
+
+ }
+
+ method.setInvocationMethod("z");
+
+ //should fail, not static
+ try {
+ new FunctionLibrary(SystemFunctionManager.getSystemFunctions(), new FunctionTree(new UDFSource(Arrays.asList(method)), true));
+ fail();
+ } catch (TeiidRuntimeException e) {
+
+ }
- // Wrap method in a list
- List methods = new ArrayList();
- methods.add(method);
- return methods;
- }
-
- public Class getInvocationClass(String className) throws ClassNotFoundException {
- throw new ClassNotFoundException("Could not find class " + className); //$NON-NLS-1$
- }
- };
+ method.setInvocationMethod("y");
- new FunctionLibrary(SystemFunctionManager.getSystemFunctions(), new FunctionTree(new UDFSource(dummySource.getFunctionMethods())));
+ //valid!
+ new FunctionLibrary(SystemFunctionManager.getSystemFunctions(), new FunctionTree(new UDFSource(Arrays.asList(method)), true));
}
- public void testNullCategory() {
+ @Test public void testNullCategory() {
FunctionMetadataSource fms = Mockito.mock(FunctionMetadataSource.class);
Mockito.stub(fms.getFunctionMethods()).toReturn(Arrays.asList(new FunctionMethod(
- "dummy", null, null, FunctionMethod.MUST_PUSHDOWN, "nonexistentClass", "noMethod", //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$
+ "dummy", null, null, FunctionMethod.MUST_PUSHDOWN, "nonexistentClass", "noMethod", //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
new FunctionParameter[0],
new FunctionParameter("output", DataTypeManager.DefaultDataTypes.STRING) //$NON-NLS-1$
)));
More information about the teiid-commits
mailing list