Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(#1046): Resolve excessive class pass scanning by caching scan res… #1048

Merged
merged 1 commit into from
Nov 3, 2023
Merged

fix(#1046): Resolve excessive class pass scanning by caching scan res… #1048

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions core/citrus-api/src/main/java/org/citrusframework/message/MessageSelector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
package org.citrusframework.message;

import java.util.HashMap;
import java.util.Map;

import java.util.concurrent.ConcurrentHashMap;
import org.citrusframework.context.TestContext;
import org.citrusframework.spi.ResourcePathTypeResolver;
import org.citrusframework.spi.TypeResolver;
Expand All @@ -24,7 +24,7 @@ public interface MessageSelector {
/** Type resolver to find custom message selectors on classpath via resource path lookup */
TypeResolver TYPE_RESOLVER = new ResourcePathTypeResolver(RESOURCE_PATH);

Map<String, MessageSelectorFactory> factories = new HashMap<>();
Map<String, MessageSelectorFactory> factories = new ConcurrentHashMap<>();

/**
* Resolves all available selectors from resource path lookup. Scans classpath for validator meta information
Expand All @@ -36,7 +36,7 @@ static Map<String, MessageSelectorFactory> lookup() {
factories.putAll(TYPE_RESOLVER.resolveAll());

if (logger.isDebugEnabled()) {
factories.forEach((k, v) -> logger.debug(String.format("Found value matcher '%s' as %s", k, v.getClass())));
factories.forEach((k, v) -> logger.debug(String.format("Found message selector '%s' as %s", k, v.getClass())));
}
}
return factories;
Expand Down
236 changes: 169 additions & 67 deletions core/citrus-api/src/main/java/org/citrusframework/spi/ResourcePathTypeResolver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import java.util.Objects;
import java.util.Optional;
import java.util.Properties;
import java.util.concurrent.ConcurrentHashMap;
import java.util.stream.Stream;
import java.util.zip.ZipEntry;
import java.util.zip.ZipInputStream;
Expand All @@ -29,13 +30,18 @@
import org.slf4j.LoggerFactory;

/**
* Type resolver resolves references via resource path lookup. Provided resource paths should point to a resource in classpath
* (e.g. META-INF/my/resource/path/file-name). The resolver will try to locate the resource as classpath resource and read the file as property
* file. By default, the resolver reads the default type resolver property {@link TypeResolver#DEFAULT_TYPE_PROPERTY} and instantiates a new instance
* for the given type information.
*
* Type resolver resolves references via resource path lookup. Provided resource paths should point
* to a resource in classpath (e.g. META-INF/my/resource/path/file-name). The resolver will try to
* locate the resource as classpath resource and read the file as property file. By default, the
* resolver reads the default type resolver property {@link TypeResolver#DEFAULT_TYPE_PROPERTY} and
* instantiates a new instance for the given type information. Note that, in order to reduce classpath
* scanning, the resolver caches the results of specific classpath scans.
* <p>
* A possible property file content that represents the resource in classpath could look like this:
* type=org.citrusframework.MySpecialPojo
* <p>
* Users can define custom property names to read instead of the default
* {@link TypeResolver#DEFAULT_TYPE_PROPERTY}.
*
* Users can define custom property names to read instead of the default {@link TypeResolver#DEFAULT_TYPE_PROPERTY}.
* @author Christoph Deppisch
Expand All @@ -57,6 +63,17 @@ public class ResourcePathTypeResolver implements TypeResolver {
/** Zip entries as String, so the archive is read only once */
private final List<String> zipEntriesAsString = Collections.synchronizedList(new ArrayList<>());

/**
* Cached properties loaded from classpath scans.
*/
private final Map<String, Properties> resourceProperties = new ConcurrentHashMap<>();


/**
* Cached specifc type names as resolved from classpath.
*/
private final Map<String, Map<String, String>> typeCache = new ConcurrentHashMap<>();

/**
* Default constructor using META-INF resource base path.
*/
Expand All @@ -82,70 +99,88 @@ public String resolveProperty(String resourcePath, String property) {
}

@Override
public <T> T resolve(String resourcePath, String property, Object ... initargs) {
String type = resolveProperty(resourcePath, property);

try {
if (initargs.length == 0) {
return (T) Class.forName(type).getDeclaredConstructor().newInstance();
} else {
return (T) getConstructor(Class.forName(type), initargs).newInstance(initargs);
}
} catch (ClassNotFoundException | IllegalAccessException | InstantiationException |
NoSuchMethodException | InvocationTargetException e) {

try {
if (Arrays.stream(Class.forName(type).getFields()).anyMatch(f -> f.getName().equals(INSTANCE) &&
Modifier.isStatic(f.getModifiers()))) {
return (T) Class.forName(type).getField(INSTANCE).get(null);
}
} catch (IllegalAccessException | NoSuchFieldException | ClassNotFoundException e1) {
throw new CitrusRuntimeException(String.format("Failed to resolve classpath resource of type '%s'", type), e1);
}

logger.warn(String.format("Neither static instance nor accessible default constructor (%s) is given on type '%s'",
Arrays.toString(getParameterTypes(initargs)), type));
throw new CitrusRuntimeException(String.format("Failed to resolve classpath resource of type '%s'", type), e);
}
public <T> T resolve(String resourcePath, String property, Object... initargs) {
String cacheKey = toCacheKey(resourcePath, property, "NO_KEY_PROPERTY");

Map<String, String> map = typeCache.computeIfAbsent(
cacheKey,
key -> Collections.singletonMap(key,
resolveProperty(resourcePath, property)));
return (T) instantiateType(map.get(cacheKey), initargs);
}

@Override
public <T> Map<String, T> resolveAll(String path, String property, String keyProperty) {

Map<String, String> typeLookup = typeCache.computeIfAbsent(
toCacheKey(path, property, keyProperty), k ->
determineTypeLookup(path, property, keyProperty)
);

Map<String, T> resources = new HashMap<>();
final String fullPath = getFullResourcePath(path);
typeLookup.forEach((p, type) -> resources.put(p, (T) instantiateType(type)));

return resources;
}

/**
* Determine the type lookup by performing relevant classpath scans.
*/
private Map<String, String> determineTypeLookup(String path, String property,
String keyProperty) {
Map<String, String> typeLookup = new HashMap<>();

final String fullPath = getFullResourcePath(path);
try {
Stream.concat(
classpathResourceResolver.getResources(fullPath).stream().filter(Objects::nonNull),
classpathResourceResolver.getResources(fullPath).stream()
.filter(Objects::nonNull),
resolveAllFromJar(fullPath))
.forEach(resourcePath -> {
Path fileName = resourcePath.getFileName();
if (fileName == null) {
logger.warn(String.format("Skip unsupported resource '%s' for resource lookup", resourcePath));
return;
}
.forEach(resourcePath -> {
Path fileName = resourcePath.getFileName();
if (fileName == null) {
logger.warn(String.format(
"Skip unsupported resource '%s' for resource lookup",
resourcePath));
return;
}

if (property.equals(TYPE_PROPERTY_WILDCARD)) {
Properties properties = readAsProperties(fullPath + "/" + fileName);
for (Map.Entry<Object, Object> prop : properties.entrySet()) {
T resource = resolve(fullPath + "/" + fileName, prop.getKey().toString());
resources.put(fileName + "." + prop.getKey().toString(), resource);
}
if (property.equals(TYPE_PROPERTY_WILDCARD)) {
Properties properties = readAsProperties(fullPath + "/" + fileName);
for (Map.Entry<Object, Object> prop : properties.entrySet()) {
String type =
resolveProperty(fullPath + "/" + fileName,
prop.getKey().toString());
typeLookup.put(fileName + "." + prop.getKey().toString(),
type);
}
} else {
String type =
resolveProperty(fullPath + "/" + fileName, property);
if (keyProperty != null) {
typeLookup.put(
resolveProperty(fullPath + "/" + fileName, keyProperty),
type);
} else {
T resource = resolve(fullPath + "/" + fileName, property);

if (keyProperty != null) {
resources.put(resolveProperty(fullPath + "/" + fileName, keyProperty), resource);
} else {
resources.put(fileName.toString(), resource);
}
typeLookup.put(fileName.toString(), type);
}
});
}
});
} catch (IOException e) {
logger.warn(String.format("Failed to resolve resources in '%s'", fullPath), e);
}

return resources;
return typeLookup;
}

private String toCacheKey(String path, String property, String keyProperty) {
StringBuilder builder = new StringBuilder();
builder.append(path);
builder.append("$$$");
builder.append(property);
builder.append("$$$");
builder.append(keyProperty);
return builder.toString();
}

private Stream<Path> resolveAllFromJar(String path) {
Expand Down Expand Up @@ -199,7 +234,7 @@ private Constructor<?> getConstructor(Class<?> type, Object[] initargs) {
final Class<?>[] parameterTypes = getParameterTypes(initargs);

Optional<Constructor<?>> exactMatch = Arrays.stream(type.getDeclaredConstructors())
.filter(constructor -> Arrays.equals(constructor.getParameterTypes(), parameterTypes))
.filter(constructor -> Arrays.equals(replacePrimitiveTypes(constructor), parameterTypes))
.findFirst();

if (exactMatch.isPresent()) {
Expand Down Expand Up @@ -232,25 +267,32 @@ private Constructor<?> getConstructor(Class<?> type, Object[] initargs) {

/**
* Read resource from classpath and load content as properties.
* The properties found on the classpath will be cached.
*
* @param resourcePath
* @return
*/
private Properties readAsProperties(String resourcePath) {
String path = getFullResourcePath(resourcePath);

InputStream in = ResourcePathTypeResolver.class.getClassLoader().getResourceAsStream(path);
if (in == null) {
throw new CitrusRuntimeException(String.format("Failed to locate resource path '%s'", path));
}
return resourceProperties.computeIfAbsent(resourcePath, k -> {
String path = getFullResourcePath(resourcePath);

InputStream in = ResourcePathTypeResolver.class.getClassLoader()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find it very hard to read, to be honest. its' probably IntelliJ auto-formatting and I know we have no prettier or any other rules in this project.

but, while looking through it I did at least find this line: this results in open handles.

I will follow up with another PR, reformatting this (and any other issues I might find).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see 3443c8b in #1044.

.getResourceAsStream(path);
if (in == null) {
throw new CitrusRuntimeException(
String.format("Failed to locate resource path '%s'", path));
}

try {
Properties config = new Properties();
config.load(in);
try {
Properties config = new Properties();
config.load(in);

return config;
} catch (IOException e) {
throw new CitrusRuntimeException(String.format("Unable to load properties from resource path configuration at '%s'", path), e);
}
return config;
} catch (IOException e) {
throw new CitrusRuntimeException(String.format(
"Unable to load properties from resource path configuration at '%s'", path), e);
}
});
}

/**
Expand All @@ -276,4 +318,64 @@ private String getFullResourcePath(String resourcePath) {
private Class<?>[] getParameterTypes(Object... initargs) {
return Arrays.stream(initargs).map(Object::getClass).toArray(Class[]::new);
}

/**
* Instantiate a type by its name.
*/
public <T> T instantiateType(String type, Object... initargs) {
try {
if (initargs.length == 0) {
return (T) Class.forName(type).getDeclaredConstructor().newInstance();
} else {
return (T) getConstructor(Class.forName(type), initargs).newInstance(initargs);
}
} catch (ClassNotFoundException | IllegalAccessException | InstantiationException |
NoSuchMethodException | InvocationTargetException e) {

try {
if (Arrays.stream(Class.forName(type).getFields())
.anyMatch(f -> f.getName().equals(INSTANCE) &&
Modifier.isStatic(f.getModifiers()))) {
return (T) Class.forName(type).getField(INSTANCE).get(null);
}
} catch (IllegalAccessException | NoSuchFieldException |
ClassNotFoundException e1) {
throw new CitrusRuntimeException(
String.format("Failed to resolve classpath resource of type '%s'", type),
e1);
}

logger.warn(String.format(
"Neither static instance nor accessible default constructor (%s) is given on type '%s'",
Arrays.toString(getParameterTypes(initargs)), type));
throw new CitrusRuntimeException(
String.format("Failed to resolve classpath resource of type '%s'", type), e);
}

}

/**
* Get the types of a constructor. Primitive types are converted to their respective object type.
* @param constructor
* @return the types of the constructor (primitive types converted to object types)
*/
private static Class<?>[] replacePrimitiveTypes(Constructor<?> constructor) {
Class<?>[] constructorParameters = constructor.getParameterTypes();
for (int i=0;i<constructorParameters.length;i++) {
if (constructorParameters[i] == int.class) {
constructorParameters[i] = Integer.class;
} else if (constructorParameters[i] == short.class) {
constructorParameters[i] = Short.class;
} else if (constructorParameters[i] == double.class) {
constructorParameters[i] = Double.class;
} else if (constructorParameters[i] == float.class) {
constructorParameters[i] = Float.class;
} else if (constructorParameters[i] == char.class) {
constructorParameters[i] = Character.class;
} else if (constructorParameters[i] == boolean.class) {
constructorParameters[i] = Boolean.class;
}
}
return constructorParameters;
}
}
6 changes: 3 additions & 3 deletions core/citrus-api/src/main/java/org/citrusframework/validation/ValueMatcher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
package org.citrusframework.validation;

import java.util.HashMap;
import java.util.Map;
import java.util.Optional;

import java.util.concurrent.ConcurrentHashMap;
import org.citrusframework.context.TestContext;
import org.citrusframework.exceptions.CitrusRuntimeException;
import org.citrusframework.spi.ResourcePathTypeResolver;
Expand All @@ -25,7 +25,7 @@ public interface ValueMatcher {
/** Type resolver to find custom message validators on classpath via resource path lookup */
TypeResolver TYPE_RESOLVER = new ResourcePathTypeResolver(RESOURCE_PATH);

Map<String, ValueMatcher> validators = new HashMap<>();
Map<String, ValueMatcher> validators = new ConcurrentHashMap<>();

/**
* Resolves all available validators from resource path lookup. Scans classpath for validator meta information
Expand All @@ -37,7 +37,7 @@ static Map<String, ValueMatcher> lookup() {
validators.putAll(TYPE_RESOLVER.resolveAll());

if (logger.isDebugEnabled()) {
validators.forEach((k, v) -> logger.debug(String.format("Found value matcher '%s' as %s", k, v.getClass())));
validators.forEach((k, v) -> logger.debug(String.format("Found validator '%s' as %s", k, v.getClass())));
}
}
return validators;
Expand Down
Loading
Loading