Skip to content

Commit

Permalink
fix: re-engineer how calls work in the engine (#7152)
Browse files Browse the repository at this point in the history 
* docs: update docs wrt engine updates #7064

Signed-off-by: jgomer2001 <bonustrack310@gmail.com>

* test: update and augment test cases and related resources #7064 #6997

Signed-off-by: jgomer2001 <bonustrack310@gmail.com>

* fix: revamp method selection & argument conversion strategy #7064

Signed-off-by: jgomer2001 <bonustrack310@gmail.com>

---------

Signed-off-by: jgomer2001 <bonustrack310@gmail.com>
Signed-off-by: Mustafa Baser <mbaser@mail.com>
  • Loading branch information
@jgomer2001 @devrimyatar
jgomer2001 authored and devrimyatar committed Dec 30, 2023
1 parent 3c04191 commit ddc2d12
Show file tree
Hide file tree
Showing 16 changed files with 1,096 additions and 459 deletions.
2 changes: 1 addition & 1 deletion agama/transpiler/src/main/resources/util.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ function _equals(a, b) {
}

function _actionCall(instance, clsName, method, args) {
return _scriptUtils.callAction(instance, clsName, method, args.map(_scan))
return _scriptUtils.callAction(_scan(instance), clsName, method, args.map(_scan))
}

function _flowCall(flowName, basePath, urlOverrides, args) {
Expand Down
2 changes: 1 addition & 1 deletion docs/admin/developer/agama/faq.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ This can be configured in the project's [metadata file](../../../agama/gama-form

### Updates in a flow's code are not reflected in its execution

When a project is re-deployed, a flow remains unchanged if it comes with errors in the provided `.gama` file. The response obtained by the deployment API will let you know which flows have errors and their descriptions.
When a project is re-deployed, a flow remains unchanged if it comes with errors in the provided `.gama` file. The tool used for deployment will let you know which flows have errors and their descriptions.

### Why are the contents of a list or map logged partially?

Expand Down
9 changes: 4 additions & 5 deletions docs/admin/developer/agama/jans-agama-engine.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -232,9 +232,11 @@ The usage of a hash sign (or spaces) before a method name helps disambiguate whe

Any method that meets the conditions mentioned (public or interface static) and that is reachable in the JVM [classpath](#classpath) can be called; developers are not restricted solely to `java.*` packages.

When using `Call`, the method to execute is picked based on the name (e.g. after the `#` sign) and the number of arguments supplied. If a class/interface exhibits several methods with the same name and arity (number of parameters), there is **no way to know** which of the available variants will be called. The `java.util.Arrays` class has several methods of this kind for instance.
When using `Call`, the method to execute is picked based on the name (e.g. after the `#` sign) and the number of arguments supplied. If a class/interface exhibits several methods with the same name and arity (number of parameters), the method that best matches the dataypes of the arguments with respect to its signature is selected. Sometimes this requires to perform arguments [conversions](#arguments-conversion) and they may fail. In such case, the second best suited method is tried and so on.

For non-static method invocations, i.e. no hash sign, the class used for method lookup is that of the instance passed (the first parameter in the `Call` directive). When the instance does not hold a Java but an Agama value, the following is used to pick a class:
When all attempts fail or there are no candidate methods to choose from, the `Call` simply throws a `NoSuchMethodException`.

For non-static method invocations, i.e. no hash sign, the class used for method lookup is that of the instance passed (the first parameter in the `Call` directive). This includes all associated superclasses too, as expected. When the instance does not hold a Java but an Agama value, the following is used to pick a class:

|Agama type|Java class for method lookup|
|-|-|
Expand All @@ -244,9 +246,6 @@ For non-static method invocations, i.e. no hash sign, the class used for method
|`list`|`java.util.List`|
|`map`|`java.util.Map`|

Once a concrete method is selected, a best effort is made to convert (if required) the values passed as arguments so that they match the expected parameter types in the method signature. If a conversion fails, this will degenerate in an `IllegalArgumentException`. More on conversions [here](#arguments-conversion).


**Limitations:**

- _list_ and _map_ literals cannot be passed as arguments to method calls directly. This means the following is illegal: `Call co.Utils#myMethod { key: [ 1, 2 , 3] } [ "Yeeha!" ]`. To achieve the same effect assign the literal value to a variable and pass that instead
Expand Down
180 changes: 8 additions & 172 deletions jans-auth-server/agama/engine/src/main/java/io/jans/agama/engine/service/ActionService.java
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
package io.jans.agama.engine.service;

import com.fasterxml.jackson.databind.DeserializationFeature;
import com.fasterxml.jackson.databind.JavaType;
import com.fasterxml.jackson.databind.ObjectMapper;

import groovy.lang.GroovyClassLoader;
import groovy.util.GroovyScriptEngine;
import groovy.util.ResourceException;
Expand All @@ -13,24 +9,13 @@
import jakarta.enterprise.context.ApplicationScoped;
import jakarta.inject.Inject;
import java.io.File;
import java.lang.reflect.Constructor;
import java.lang.reflect.Executable;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.lang.reflect.Parameter;
import java.lang.reflect.ParameterizedType;
import java.lang.reflect.Type;
import java.net.MalformedURLException;
import java.net.URL;
import java.util.Arrays;
import java.util.List;
import java.util.Optional;
import java.util.function.BiPredicate;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import io.jans.agama.engine.misc.PrimitiveUtils;
import io.jans.agama.model.EngineConfig;

import org.codehaus.groovy.control.CompilerConfiguration;
Expand All @@ -47,8 +32,10 @@ public class ActionService {
@Inject
private EngineConfig econf;

@Inject
private MethodInvoker invoker;

private GroovyScriptEngine gse;
private ObjectMapper mapper;
private GroovyClassLoader loader;

public Object callAction(Object instance, String className, String methodName, Object[] rhinoArgs)
Expand All @@ -57,7 +44,7 @@ public Object callAction(Object instance, String className, String methodName, O
boolean noInst = instance == null;
Class actionCls = null;

if (!noInst) {
if (!noInst) {
actionCls = instance.getClass();
className = actionCls.getName();
} else {
Expand Down Expand Up @@ -90,166 +77,22 @@ public Object callAction(Object instance, String className, String methodName, O
}
}
logger.debug("Class {} loaded successfully", className);
int arity = rhinoArgs.length;

BiPredicate<Executable, Boolean> pr = (e, staticRequired) -> {
int mod = e.getModifiers();
return e.getParameterCount() == arity && Modifier.isPublic(mod) &&
(staticRequired ? Modifier.isStatic(mod) : true);
};

//Search for a method/constructor matching name and arity
//Search for the best matching method/constructor

if (noInst) {
if (methodName.equals("new")) {
Constructor constr = Stream.of(actionCls.getConstructors()).filter(c -> pr.test(c, false))
.findFirst().orElse(null);
if (constr == null) {
String msg = String.format("Unable to find a constructor with arity %d in class %s",
arity, className);
logger.error(msg);
throw new InstantiationException(msg);
}
if (methodName.equals("new")) return invoker.callConstructor(actionCls, rhinoArgs);

if (logger.isDebugEnabled()) {
logger.debug("Constructor found: {}", constr.toGenericString());
}
Object[] args = getArgsForCall(constr, arity, rhinoArgs);

logger.debug("Creating an instance");
return constr.newInstance(args);

} else if (methodName.equals("class")) {
if (methodName.equals("class")) {
logger.debug("Returning class object");
return actionCls;
}
}

Method javaMethod = Stream.of(actionCls.getDeclaredMethods())
.filter(m -> m.getName().equals(methodName)).filter(m -> pr.test(m, noInst))
.findFirst().orElse(null);

if (javaMethod == null) {
String msg = String.format("Unable to find a method called %s with arity %d in class %s",
methodName, arity, className);
logger.error(msg);
throw new NoSuchMethodException(msg);
}

if (logger.isDebugEnabled()) {
logger.debug("Method found: {}", javaMethod.toGenericString());
}
Object[] args = getArgsForCall(javaMethod, arity, rhinoArgs);

try {
logger.debug("Performing method call");
return javaMethod.invoke(instance, args);
} catch (InvocationTargetException e) {
throw (Exception) e.getCause(); //return the "real" exception, e is just a wrapper here
}
return invoker.call(actionCls, instance, methodName, rhinoArgs);

}

private Object[] getArgsForCall(Executable javaExec, int arity, Object[] arguments)
throws IllegalArgumentException {

Object[] javaArgs = new Object[arity];
int i = -1;

for (Parameter p : javaExec.getParameters()) {
Object arg = arguments[++i];
Class<?> paramType = p.getType();
String typeName = paramType.getName();
logger.debug("Examining argument at index {}", i);

if (arg == null) {
logger.debug("Value is null");
if (PrimitiveUtils.isPrimitive(paramType, false))
throw new IllegalArgumentException("null value passed for a primitive parameter of type "
+ typeName);
else continue;
}
if (typeName.equals(Object.class.getName())) {
//This parameter can receive anything :(
logger.trace("Parameter is a {}", typeName);
javaArgs[i] = arg;
continue;
}

//argClass does not carry type information due to Java type erasure
Class<?> argClass = arg.getClass();

//Try to apply cheaper conversions first (in comparison to mapper-based conversion)
//Note: A numeric literal coming from Javascript code lands as a Double
Boolean primCompat = PrimitiveUtils.compatible(argClass, paramType);
if (primCompat != null) {

if (primCompat) {
logger.trace("Parameter is a primitive (or wrapped) {}", typeName);
javaArgs[i] = arg;

} else if (Number.class.isAssignableFrom(argClass)) {
Object number = PrimitiveUtils.primitiveNumberFrom((Number) arg, paramType);

if (number != null) {
logger.trace("Parameter is a primitive (or wrapped) {}", typeName);
javaArgs[i] = number;

} else mismatchError(argClass, typeName);

} else mismatchError(argClass, typeName);

} else if (CharSequence.class.isAssignableFrom(argClass)) {

primCompat = PrimitiveUtils.compatible(Character.class, paramType);

if (Optional.ofNullable(primCompat).orElse(false)) {
int len = arg.toString().length();
if (len == 0 || len > 1) mismatchError(argClass, typeName);

logger.trace("Parameter is a {}", typeName);
javaArgs[i] = arg.toString().charAt(0);

} else if (paramType.isAssignableFrom(argClass)) {
logger.trace("Parameter is a {}", typeName);
javaArgs[i] = arg;

} else mismatchError(argClass, typeName);

} else {
//argClass should be NativeArray or NativeObject if the value was not created/derived
//from a Java call
Type parameterizedType = p.getParameterizedType();
String ptypeName = parameterizedType.getTypeName();

boolean straight = false;
if (paramType.isInstance(arg)) {
//ptypeName and typeName are equal when there is no type information in the parameter
//(method signature). For instance: String[], List, MyBean (no parameterized types)
straight = ptypeName.equals(typeName);

if (!straight && ParameterizedType.class.isInstance(parameterizedType)) {
//make straight assignment if all type params are like <?,?,...>
straight = Stream.of(((ParameterizedType) parameterizedType).getActualTypeArguments())
.map(Type::getTypeName).allMatch("?"::equals);
}
}

if (straight) {
javaArgs[i] = arg;
} else {
logger.warn("Trying to parse argument of class {} to {}", argClass.getCanonicalName(), ptypeName);

JavaType javaType = mapper.getTypeFactory().constructType(parameterizedType);
javaArgs[i] = mapper.convertValue(arg, javaType);
}
logger.trace("Parameter is a {}", ptypeName);
}
}
return javaArgs;

}

public GroovyClassLoader getClassLoader() {
return loader;
}
Expand All @@ -258,10 +101,6 @@ public Class<?> classFromName(String qname) throws ClassNotFoundException {
return Class.forName(qname, false, loader);
}

private void mismatchError(Class<?> argClass, String typeName) throws IllegalArgumentException {
throw new IllegalArgumentException(argClass.getSimpleName() + " passed for a " + typeName);
}

@PostConstruct
private void init() {

Expand All @@ -283,9 +122,6 @@ private void init() {
loader = gse.getGroovyClassLoader();
loader.setShouldRecompile(true);

mapper = new ObjectMapper();
mapper.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);

}

}
Loading

0 comments on commit ddc2d12

Please sign in to comment.