Skip to content

Commit

Permalink
[FLINK-17660][table-common] Check default constructor for structured …
Browse files Browse the repository at this point in the history 
…types

This closes apache#12123.
  • Loading branch information
@twalthr
twalthr committed May 13, 2020
1 parent 47323a4 commit 66a0dd8
Show file tree
Hide file tree
Showing 5 changed files with 176 additions and 110 deletions.
7 changes: 7 additions & 0 deletions ...table-common/src/main/java/org/apache/flink/table/types/extraction/DataTypeExtractor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
import static org.apache.flink.table.types.extraction.ExtractionUtils.createRawType;
import static org.apache.flink.table.types.extraction.ExtractionUtils.extractAssigningConstructor;
import static org.apache.flink.table.types.extraction.ExtractionUtils.extractionError;
import static org.apache.flink.table.types.extraction.ExtractionUtils.hasInvokableConstructor;
import static org.apache.flink.table.types.extraction.ExtractionUtils.isStructuredFieldMutable;
import static org.apache.flink.table.types.extraction.ExtractionUtils.resolveVariable;
import static org.apache.flink.table.types.extraction.ExtractionUtils.toClass;
Expand Down Expand Up @@ -475,6 +476,12 @@ private DataType extractStructuredType(DataTypeTemplate template, List<Type> typ
clazz.getName(),
fields.stream().map(Field::getName).collect(Collectors.joining(", ")));
}
// check for a default constructor otherwise
else if (constructor == null && !hasInvokableConstructor(clazz)) {
throw extractionError(
"Class '%s' has neither a constructor that assigns all fields nor a default constructor.",
clazz.getName());
}

final Map<String, DataType> fieldDataTypes = extractStructuredTypeFields(
template,
Expand Down
242 changes: 139 additions & 103 deletions ...k-table-common/src/main/java/org/apache/flink/table/types/extraction/ExtractionUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
Expand All @@ -66,6 +67,10 @@
@Internal
public final class ExtractionUtils {

// --------------------------------------------------------------------------------------------
// Methods shared across packages
// --------------------------------------------------------------------------------------------

/**
* Collects methods of the given name.
*/
Expand All @@ -77,29 +82,33 @@ public static List<Method> collectMethods(Class<?> function, String methodName)
}

/**
* Checks whether a method can be called with the given argument classes. This includes type
* Checks whether a method/constructor can be called with the given argument classes. This includes type
* widening and vararg. {@code null} is a wildcard.
*
* <p>E.g., {@code (int.class, int.class)} matches {@code f(Object...), f(int, int), f(Integer, Object)}
* and so forth.
*/
public static boolean isMethodInvokable(Method method, Class<?>... classes) {
final int paramCount = method.getParameterCount();
public static boolean isInvokable(Executable executable, Class<?>... classes) {
final int m = executable.getModifiers();
if (!Modifier.isPublic(m)) {
return false;
}
final int paramCount = executable.getParameterCount();
final int classCount = classes.length;
// check for enough classes for each parameter
if (classCount < paramCount || (method.isVarArgs() && classCount < paramCount - 1)) {
if (classCount < paramCount || (executable.isVarArgs() && classCount < paramCount - 1)) {
return false;
}
int currentClass = 0;
for (int currentParam = 0; currentParam < paramCount; currentParam++) {
final Class<?> param = method.getParameterTypes()[currentParam];
final Class<?> param = executable.getParameterTypes()[currentParam];
// entire parameter matches
if (classes[currentClass] == null || ExtractionUtils.isAssignable(classes[currentClass], param, true)) {
currentClass++;
}
// last parameter is a vararg that consumes remaining classes
else if (currentParam == paramCount - 1 && method.isVarArgs()) {
final Class<?> paramComponent = method.getParameterTypes()[currentParam].getComponentType();
else if (currentParam == paramCount - 1 && executable.isVarArgs()) {
final Class<?> paramComponent = executable.getParameterTypes()[currentParam].getComponentType();
while (currentClass < classCount && ExtractionUtils.isAssignable(classes[currentClass], paramComponent, true)) {
currentClass++;
}
Expand Down Expand Up @@ -136,6 +145,125 @@ public static String createMethodSignatureString(
return builder.toString();
}

/**
* Checks for a field getter of a structured type. The logic is as broad as possible to support
* both Java and Scala in different flavors.
*/
public static Optional<Method> getStructuredFieldGetter(Class<?> clazz, Field field) {
final String normalizedFieldName = field.getName().toUpperCase();

final List<Method> methods = collectStructuredMethods(clazz);
for (Method method : methods) {
// check name:
// get<Name>()
// is<Name>()
// <Name>() for Scala
final String normalizedMethodName = method.getName().toUpperCase();
final boolean hasName = normalizedMethodName.equals("GET" + normalizedFieldName) ||
normalizedMethodName.equals("IS" + normalizedFieldName) ||
normalizedMethodName.equals(normalizedFieldName);
if (!hasName) {
continue;
}

// check return type:
// equal to field type
final Type returnType = method.getGenericReturnType();
final boolean hasReturnType = returnType.equals(field.getGenericType());
if (!hasReturnType) {
continue;
}

// check parameters:
// no parameters
final boolean hasNoParameters = method.getParameterCount() == 0;
if (!hasNoParameters) {
continue;
}

// matching getter found
return Optional.of(method);
}

// no getter found
return Optional.empty();
}

/**
* Checks for a field setters of a structured type. The logic is as broad as possible to support
* both Java and Scala in different flavors.
*/
public static Optional<Method> getStructuredFieldSetter(Class<?> clazz, Field field) {
final String normalizedFieldName = field.getName().toUpperCase();

final List<Method> methods = collectStructuredMethods(clazz);
for (Method method : methods) {

// check name:
// set<Name>(type)
// <Name>(type)
// <Name>_$eq(type) for Scala
final String normalizedMethodName = method.getName().toUpperCase();
final boolean hasName = normalizedMethodName.equals("SET" + normalizedFieldName) ||
normalizedMethodName.equals(normalizedFieldName) ||
normalizedMethodName.equals(normalizedFieldName + "_$EQ");
if (!hasName) {
continue;
}

// check return type:
// void or the declaring class
final Class<?> returnType = method.getReturnType();
final boolean hasReturnType = returnType == Void.TYPE || returnType == clazz;
if (!hasReturnType) {
continue;
}

// check parameters:
// one parameter that has the same (or primitive) type of the field
final boolean hasParameter = method.getParameterCount() == 1 &&
(method.getGenericParameterTypes()[0].equals(field.getGenericType()) ||
primitiveToWrapper(method.getGenericParameterTypes()[0]).equals(field.getGenericType()));
if (!hasParameter) {
continue;
}

// matching setter found
return Optional.of(method);
}

// no setter found
return Optional.empty();
}

/**
* Checks for an invokable constructor matching the given arguments.
*
* @see #isInvokable(Executable, Class[])
*/
public static boolean hasInvokableConstructor(Class<?> clazz, Class<?>... classes) {
for (Constructor<?> constructor : clazz.getDeclaredConstructors()) {
if (isInvokable(constructor, classes)) {
return true;
}
}
return false;
}

/**
* Checks whether a field is directly readable without a getter.
*/
public static boolean isStructuredFieldDirectlyReadable(Field field) {
final int m = field.getModifiers();

// field is directly readable
return Modifier.isPublic(m);
}

// --------------------------------------------------------------------------------------------
// Methods intended for this package
// --------------------------------------------------------------------------------------------

/**
* Helper method for creating consistent exceptions during extraction.
*/
Expand Down Expand Up @@ -310,15 +438,13 @@ static List<Field> collectStructuredFields(Class<?> clazz) {
* Validates if a field is properly readable either directly or through a getter.
*/
static void validateStructuredFieldReadability(Class<?> clazz, Field field) {
final int m = field.getModifiers();

// field is accessible
if (Modifier.isPublic(m)) {
if (isStructuredFieldDirectlyReadable(field)) {
return;
}

// field needs a getter
if (!hasStructuredFieldGetter(clazz, field)) {
if (!getStructuredFieldGetter(clazz, field).isPresent()) {
throw extractionError(
"Field '%s' of class '%s' is neither publicly accessible nor does it have " +
"a corresponding getter method.",
Expand All @@ -342,8 +468,9 @@ static boolean isStructuredFieldMutable(Class<?> clazz, Field field) {
if (Modifier.isPublic(m)) {
return true;
}

// field has setters by which it is mutable
if (hasFieldSetter(clazz, field)) {
if (getStructuredFieldSetter(clazz, field).isPresent()) {
return true;
}

Expand All @@ -354,53 +481,6 @@ static boolean isStructuredFieldMutable(Class<?> clazz, Field field) {
clazz.getName());
}

/**
* Checks for a field setters. The logic is as broad as possible to support both Java and Scala
* in different flavors.
*/
static boolean hasFieldSetter(Class<?> clazz, Field field) {
final String normalizedFieldName = field.getName().toUpperCase();

final List<Method> methods = collectStructuredMethods(clazz);
for (Method method : methods) {

// check name:
// set<Name>(type)
// <Name>(type)
// <Name>_$eq(type) for Scala
final String normalizedMethodName = method.getName().toUpperCase();
final boolean hasName = normalizedMethodName.equals("SET" + normalizedFieldName) ||
normalizedMethodName.equals(normalizedFieldName) ||
normalizedMethodName.equals(normalizedFieldName + "_$EQ");
if (!hasName) {
continue;
}

// check return type:
// void or the declaring class
final Class<?> returnType = method.getReturnType();
final boolean hasReturnType = returnType == Void.TYPE || returnType == clazz;
if (!hasReturnType) {
continue;
}

// check parameters:
// one parameter that has the same (or primitive) type of the field
final boolean hasParameter = method.getParameterCount() == 1 &&
(method.getGenericParameterTypes()[0].equals(field.getGenericType()) ||
primitiveToWrapper(method.getGenericParameterTypes()[0]).equals(field.getGenericType()));
if (!hasParameter) {
continue;
}

// matching setter found
return true;
}

// no setter found
return false;
}

/**
* Returns the boxed type of a primitive type.
*/
Expand All @@ -411,50 +491,6 @@ static Type primitiveToWrapper(Type type) {
return type;
}

/**
* Checks for a field getter. The logic is as broad as possible to support both Java and Scala
* in different flavors.
*/
static boolean hasStructuredFieldGetter(Class<?> clazz, Field field) {
final String normalizedFieldName = field.getName().toUpperCase();

final List<Method> methods = collectStructuredMethods(clazz);
for (Method method : methods) {
// check name:
// get<Name>()
// is<Name>()
// <Name>() for Scala
final String normalizedMethodName = method.getName().toUpperCase();
final boolean hasName = normalizedMethodName.equals("GET" + normalizedFieldName) ||
normalizedMethodName.equals("IS" + normalizedFieldName) ||
normalizedMethodName.equals(normalizedFieldName);
if (!hasName) {
continue;
}

// check return type:
// equal to field type
final Type returnType = method.getGenericReturnType();
final boolean hasReturnType = returnType.equals(field.getGenericType());
if (!hasReturnType) {
continue;
}

// check parameters:
// no parameters
final boolean hasNoParameters = method.getParameterCount() == 0;
if (!hasNoParameters) {
continue;
}

// matching getter found
return true;
}

// no getter found
return false;
}

/**
* Collects all methods that qualify as methods of a {@link StructuredType}.
*/
Expand Down
8 changes: 4 additions & 4 deletions ...ommon/src/main/java/org/apache/flink/table/types/extraction/FunctionMappingExtractor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
import static org.apache.flink.table.types.extraction.ExtractionUtils.createMethodSignatureString;
import static org.apache.flink.table.types.extraction.ExtractionUtils.extractionError;
import static org.apache.flink.table.types.extraction.ExtractionUtils.isAssignable;
import static org.apache.flink.table.types.extraction.ExtractionUtils.isMethodInvokable;
import static org.apache.flink.table.types.extraction.ExtractionUtils.isInvokable;
import static org.apache.flink.table.types.extraction.TemplateUtils.extractGlobalFunctionTemplates;
import static org.apache.flink.table.types.extraction.TemplateUtils.extractLocalFunctionTemplates;
import static org.apache.flink.table.types.extraction.TemplateUtils.findInputOnlyTemplates;
Expand Down Expand Up @@ -419,7 +419,7 @@ static MethodVerification createParameterAndReturnTypeVerification() {
return (method, signature, result) -> {
final Class<?>[] parameters = signature.toArray(new Class[0]);
final Class<?> returnType = method.getReturnType();
final boolean isValid = isMethodInvokable(method, parameters) &&
final boolean isValid = isInvokable(method, parameters) &&
isAssignable(result, returnType, true);
if (!isValid) {
throw createMethodNotFoundError(method.getName(), parameters, result);
Expand Down Expand Up @@ -449,7 +449,7 @@ static MethodVerification createParameterWithArgumentVerification(@Nullable Clas
return (method, signature, result) -> {
final Class<?>[] parameters = Stream.concat(Stream.of(argumentClass), signature.stream())
.toArray(Class<?>[]::new);
if (!isMethodInvokable(method, parameters)) {
if (!isInvokable(method, parameters)) {
throw createMethodNotFoundError(method.getName(), parameters, null);
}
};
Expand All @@ -461,7 +461,7 @@ static MethodVerification createParameterWithArgumentVerification(@Nullable Clas
static MethodVerification createParameterVerification() {
return (method, signature, result) -> {
final Class<?>[] parameters = signature.toArray(new Class[0]);
if (!isMethodInvokable(method, parameters)) {
if (!isInvokable(method, parameters)) {
throw createMethodNotFoundError(method.getName(), parameters, null);
}
};
Expand Down
Loading

0 comments on commit 66a0dd8

Please sign in to comment.