Skip to content

Commit

Permalink
Optimize field annotation lookup (#1243)
Browse files Browse the repository at this point in the history 
* Optimize field annotation lookup

* javadoc

* Cleanup getCheck

* add tests

* Cleanup

* getMethodAnnotation

Co-authored-by: wcekan <wcekan@verizonmedia.com>
  • Loading branch information
@wcekan
wcekan and wcekan committed Apr 1, 2020
1 parent c7ee885 commit 912b246
Show file tree
Hide file tree
Showing 5 changed files with 81 additions and 47 deletions.
22 changes: 21 additions & 1 deletion elide-core/src/main/java/com/yahoo/elide/core/EntityBinding.java
View file
Expand Up @@ -117,7 +117,7 @@ public class EntityBinding {
public final ConcurrentHashMap<String, String> aliasesToFields = new ConcurrentHashMap<>();
public final ConcurrentHashMap<Method, Boolean> requestScopeableMethods = new ConcurrentHashMap<>();

public final ConcurrentHashMap<Class<? extends Annotation>, Annotation> annotations = new ConcurrentHashMap<>();
public final ConcurrentHashMap<Object, Annotation> annotations = new ConcurrentHashMap<>();

public static final EntityBinding EMPTY_BINDING = new EntityBinding();
private static final String ALL_FIELDS = "*";
Expand Down Expand Up @@ -572,6 +572,26 @@ public <A extends Annotation> A getAnnotation(Class<A> annotationClass) {
return annotation == NO_ANNOTATION ? null : annotationClass.cast(annotation);
}

/**
* Return annotation for provided method.
*
* @param annotationClass the annotation class
* @param method the method
* @param <A> annotation type
* @return the annotation
*/
public <A extends Annotation> A getMethodAnnotation(Class<A> annotationClass, String method) {
Annotation annotation = annotations.computeIfAbsent(Pair.of(annotationClass, method), key -> {
try {
return Optional.ofNullable((Annotation) entityClass.getMethod(method).getAnnotation(annotationClass))
.orElse(NO_ANNOTATION);
} catch (NoSuchMethodException | SecurityException e) {
throw new IllegalStateException(e);
}
});
return annotation == NO_ANNOTATION ? null : annotationClass.cast(annotation);
}

private List<Class<?>> getInheritedTypes(Class<?> entityClass) {
ArrayList<Class<?>> results = new ArrayList<>();

Expand Down
44 changes: 20 additions & 24 deletions elide-core/src/main/java/com/yahoo/elide/core/EntityDictionary.java
View file
Expand Up @@ -140,12 +140,7 @@ private static Package getParentPackage(Package pkg) {
* @return simple name
*/
public static String getSimpleName(Class<?> cls) {
String simpleName = SIMPLE_NAMES.get(cls);
if (simpleName == null) {
simpleName = cls.getSimpleName();
SIMPLE_NAMES.putIfAbsent(cls, simpleName);
}
return simpleName;
return SIMPLE_NAMES.computeIfAbsent(cls, key -> cls.getSimpleName());
}

/**
Expand Down Expand Up @@ -285,24 +280,14 @@ public ParseTree getPermissionsForField(Class<?> resourceClass,
* @return the {@link Check} mapped to the identifier or {@code null} if the given identifer is unmapped
*/
public Class<? extends Check> getCheck(String checkIdentifier) {
Class<? extends Check> checkCls = checkNames.get(checkIdentifier);

if (checkCls == null) {
return checkNames.computeIfAbsent(checkIdentifier, cls -> {
try {
checkCls = Class.forName(checkIdentifier).asSubclass(Check.class);
try {
checkNames.putIfAbsent(checkIdentifier, checkCls);
} catch (IllegalArgumentException e) {
log.error("HELP! {} {} {}", checkIdentifier, checkCls, checkNames.inverse().get(checkCls));
throw e;
}
return Class.forName(checkIdentifier).asSubclass(Check.class);
} catch (ClassNotFoundException | ClassCastException e) {
throw new IllegalArgumentException(
"Could not instantiate specified check '" + checkIdentifier + "'.", e);
}
}

return checkCls;
});
}

/**
Expand Down Expand Up @@ -345,11 +330,10 @@ public List<Class<?>> getSubclassingEntities(String entityName) {
* @return List of all inherited entity types
*/
public List<Class<?>> getSubclassingEntities(Class entityClass) {
return subclassingEntities.computeIfAbsent(entityClass, (unused) -> {
return entityBindings.keySet().stream()
.filter(c -> c != entityClass && entityClass.isAssignableFrom(c))
.collect(Collectors.toList());
});
return subclassingEntities.computeIfAbsent(entityClass, unused -> entityBindings
.keySet().stream()
.filter(c -> c != entityClass && entityClass.isAssignableFrom(c))
.collect(Collectors.toList()));
}

/**
Expand Down Expand Up @@ -894,6 +878,18 @@ public <A extends Annotation> A getAnnotation(Class<?> recordClass, Class<A> ann
return getEntityBinding(recordClass).getAnnotation(annotationClass);
}

/**
* Return annotation from class for provided method.
* @param recordClass the record class
* @param method the method
* @param annotationClass the annotation class
* @param <A> genericClass
* @return the annotation
*/
public <A extends Annotation> A getMethodAnnotation(Class<?> recordClass, String method, Class<A> annotationClass) {
return getEntityBinding(recordClass).getMethodAnnotation(annotationClass, method);
}

public <A extends Annotation> Collection<LifeCycleHook> getTriggers(Class<?> cls,
Class<A> annotationClass,
String fieldName) {
Expand Down
18 changes: 2 additions & 16 deletions elide-core/src/main/java/com/yahoo/elide/security/FilterExpressionCheck.java
View file
Expand Up @@ -91,22 +91,8 @@ public boolean applyPredicateToObject(T object, FilterPredicate filterPredicate,
*/
protected static Path getFieldPath(Class<?> type, RequestScope requestScope, String method, String defaultPath) {
EntityDictionary dictionary = coreScope(requestScope).getDictionary();
try {
FilterExpressionPath fep = getFilterExpressionPath(type, method, dictionary);
return new Path(type, dictionary, fep == null ? defaultPath : fep.value());
} catch (NoSuchMethodException | SecurityException e) {
throw new IllegalStateException(e);
}
}

private static FilterExpressionPath getFilterExpressionPath(
Class<?> type,
String method,
EntityDictionary dictionary) throws NoSuchMethodException {
FilterExpressionPath path = dictionary.lookupBoundClass(type)
.getMethod(method)
.getAnnotation(FilterExpressionPath.class);
return path;
FilterExpressionPath fep = dictionary.getMethodAnnotation(type, method, FilterExpressionPath.class);
return new Path(type, dictionary, fep == null ? defaultPath : fep.value());
}

protected static com.yahoo.elide.core.RequestScope coreScope(RequestScope requestScope) {
Expand Down
8 changes: 2 additions & 6 deletions elide-core/src/main/java/com/yahoo/elide/security/permissions/ExpressionResultCache.java
View file
Expand Up @@ -29,12 +29,8 @@ public boolean hasStoredResultFor(Class<? extends Check> checkClass, PersistentR
}

public void putResultFor(Class<? extends Check> checkClass, PersistentResource resource, ExpressionResult result) {
Map<PersistentResource, ExpressionResult> cache = computedResults.get(checkClass);
if (cache == null) {
cache = new IdentityHashMap<>();
computedResults.put(checkClass, cache);
}

Map<PersistentResource, ExpressionResult> cache = computedResults.computeIfAbsent(checkClass,
unused -> new IdentityHashMap<>());
cache.put(resource, result);
}

Expand Down
36 changes: 36 additions & 0 deletions elide-core/src/test/java/com/yahoo/elide/core/EntityDictionaryTest.java
View file
Expand Up @@ -17,6 +17,7 @@
import com.yahoo.elide.Injector;
import com.yahoo.elide.annotation.ComputedAttribute;
import com.yahoo.elide.annotation.Exclude;
import com.yahoo.elide.annotation.FilterExpressionPath;
import com.yahoo.elide.annotation.Include;
import com.yahoo.elide.annotation.MappedInterface;
import com.yahoo.elide.annotation.OnUpdatePreSecurity;
Expand Down Expand Up @@ -704,6 +705,22 @@ class Foo { }
assertTrue(first instanceof Exclude);
}

@Test
public void testAnnotationNoSuchMethod() {
bindEntity(Book.class);
IllegalStateException e = assertThrows(IllegalStateException.class,
() -> getMethodAnnotation(Book.class, "NoMethod", FilterExpressionPath.class));
assertTrue(e.getCause() instanceof NoSuchMethodException, e.toString());
}

@Test
public void testAnnotationFilterExpressionPath() {
bindEntity(Book.class);
FilterExpressionPath fe =
getMethodAnnotation(Book.class, "getEditor", FilterExpressionPath.class);
assertEquals("publisher.editor", fe.value());
}

@Test
public void testBadLookupEntityClass() {
assertThrows(IllegalArgumentException.class, () -> lookupEntityClass(null));
Expand Down Expand Up @@ -795,4 +812,23 @@ class CoerceBean {
bean.map);
assertEquals(ImmutableSet.of(3.0, 4.0), bean.set);
}

public static class TestCheck extends UserCheck {

@Override
public boolean ok(com.yahoo.elide.security.User user) {
throw new IllegalStateException();
}
}

@Test
public void testCheckLookup() throws Exception {
assertEquals(Role.ALL.class, this.getCheck("user has all access"));

assertEquals(TestCheck.class, this.getCheck("com.yahoo.elide.core.EntityDictionaryTest$TestCheck"));

assertThrows(IllegalArgumentException.class, () -> this.getCheck("UnknownClassName"));

assertThrows(IllegalArgumentException.class, () -> this.getCheck(String.class.getName()));
}
}

0 comments on commit 912b246

Please sign in to comment.