From 7048968af3a0838da51ccfcaf0d1cff468b2f9f8 Mon Sep 17 00:00:00 2001 From: William Cekan Date: Thu, 26 Mar 2020 17:16:05 -0500 Subject: [PATCH] Improve performance of initial class scans (#1238) * Improve performance of initial class scans * Limit scan to public classes Co-authored-by: wcekan --- .../src/main/java/com/yahoo/elide/Elide.java | 8 +++--- .../exceptions/UnableToAddSerdeException.java | 4 +++ .../com/yahoo/elide/utils/ClassScanner.java | 15 ++++++----- .../ElideCustomSerdeRegistrationTest.java | 26 +++++++++---------- .../elide/core/EntityDictionaryTest.java | 18 ++++++------- .../elide/core/utils/ClassScannerTest.java | 12 +++------ 6 files changed, 43 insertions(+), 40 deletions(-) diff --git a/elide-core/src/main/java/com/yahoo/elide/Elide.java b/elide-core/src/main/java/com/yahoo/elide/Elide.java index f5d3fef134..4bdd55b739 100644 --- a/elide-core/src/main/java/com/yahoo/elide/Elide.java +++ b/elide-core/src/main/java/com/yahoo/elide/Elide.java @@ -97,7 +97,7 @@ public Elide(ElideSettings elideSettings) { registerCustomSerde(); } - private void registerCustomSerde() { + protected void registerCustomSerde() { Set> classes = ClassScanner.getAnnotatedClasses(ElideTypeConverter.class); for (Class clazz : classes) { @@ -114,7 +114,7 @@ private void registerCustomSerde() { | NoSuchMethodException | InvocationTargetException e) { String errorMsg = String.format("Error while registering custom Serde: %s", e.getLocalizedMessage()); log.error(errorMsg); - throw new UnableToAddSerdeException(errorMsg); + throw new UnableToAddSerdeException(errorMsg, e); } ElideTypeConverter converter = clazz.getAnnotation(ElideTypeConverter.class); Class baseType = converter.type(); @@ -130,13 +130,13 @@ private void registerCustomSerde() { } } - private void registerCustomSerde(Class type, Serde serde, String name) { + protected void registerCustomSerde(Class type, Serde serde, String name) { log.info("Registering serde for type : {}", type); CoerceUtil.register(type, serde); registerCustomSerdeInObjectMapper(type, serde, name); } - private void registerCustomSerdeInObjectMapper(Class type, Serde serde, String name) { + protected void registerCustomSerdeInObjectMapper(Class type, Serde serde, String name) { ObjectMapper objectMapper = mapper.getObjectMapper(); objectMapper.registerModule(new SimpleModule(name) .addSerializer(type, new JsonSerializer() { diff --git a/elide-core/src/main/java/com/yahoo/elide/core/exceptions/UnableToAddSerdeException.java b/elide-core/src/main/java/com/yahoo/elide/core/exceptions/UnableToAddSerdeException.java index f9617e8a57..e793801410 100644 --- a/elide-core/src/main/java/com/yahoo/elide/core/exceptions/UnableToAddSerdeException.java +++ b/elide-core/src/main/java/com/yahoo/elide/core/exceptions/UnableToAddSerdeException.java @@ -9,4 +9,8 @@ public class UnableToAddSerdeException extends RuntimeException { public UnableToAddSerdeException(String message) { super(message); } + + public UnableToAddSerdeException(String message, Throwable cause) { + super(message, cause); + } } diff --git a/elide-core/src/main/java/com/yahoo/elide/utils/ClassScanner.java b/elide-core/src/main/java/com/yahoo/elide/utils/ClassScanner.java index f078fc90e7..767156a92b 100644 --- a/elide-core/src/main/java/com/yahoo/elide/utils/ClassScanner.java +++ b/elide-core/src/main/java/com/yahoo/elide/utils/ClassScanner.java @@ -20,7 +20,7 @@ public class ClassScanner { /** * Scans all classes accessible from the context class loader which belong to the given package and subpackages. * - * @param toScan package to scan + * @param toScan package to scan * @param annotation Annotation to search * @return The classes */ @@ -32,11 +32,12 @@ static public Set> getAnnotatedClasses(Package toScan, Class> getAnnotatedClasses(String packageName, Class annotation) { - try (ScanResult scanResult = new ClassGraph().enableAllInfo().whitelistPackages(packageName).scan()) { + try (ScanResult scanResult = new ClassGraph() + .enableClassInfo().enableAnnotationInfo().whitelistPackages(packageName).scan()) { return scanResult.getClassesWithAnnotation(annotation.getCanonicalName()).stream() .map((ClassInfo::loadClass)) .collect(Collectors.toSet()); @@ -46,11 +47,12 @@ static public Set> getAnnotatedClasses(String packageName, Class> getAnnotatedClasses(Class annotation) { - try (ScanResult scanResult = new ClassGraph().enableAllInfo().scan()) { + try (ScanResult scanResult = new ClassGraph() + .enableClassInfo().enableAnnotationInfo().scan()) { return scanResult.getClassesWithAnnotation(annotation.getCanonicalName()).stream() .map((ClassInfo::loadClass)) .collect(Collectors.toSet()); @@ -63,7 +65,8 @@ static public Set> getAnnotatedClasses(Class anno * @return All the classes within a package. */ static public Set> getAllClasses(String packageName) { - try (ScanResult scanResult = new ClassGraph().enableAllInfo().whitelistPackages(packageName).scan()) { + try (ScanResult scanResult = new ClassGraph() + .enableClassInfo().whitelistPackages(packageName).scan()) { return scanResult.getAllClasses().stream() .map((ClassInfo::loadClass)) .collect(Collectors.toSet()); diff --git a/elide-core/src/test/java/com/yahoo/elide/ElideCustomSerdeRegistrationTest.java b/elide-core/src/test/java/com/yahoo/elide/ElideCustomSerdeRegistrationTest.java index 7b5179c476..b7ab6bd5b6 100644 --- a/elide-core/src/test/java/com/yahoo/elide/ElideCustomSerdeRegistrationTest.java +++ b/elide-core/src/test/java/com/yahoo/elide/ElideCustomSerdeRegistrationTest.java @@ -24,21 +24,21 @@ class DummyTwo extends Dummy { class DummyThree extends Dummy { } -@ElideTypeConverter(type = Dummy.class, name = "Dummy", subTypes = {DummyThree.class, DummyTwo.class}) -class DummySerde implements Serde { - - @Override - public Dummy deserialize(String val) { - return null; - } - - @Override - public String serialize(Dummy val) { - return null; +public class ElideCustomSerdeRegistrationTest { + @ElideTypeConverter(type = Dummy.class, name = "Dummy", subTypes = { DummyThree.class, DummyTwo.class }) + public static class DummySerde implements Serde { + + @Override + public Dummy deserialize(String val) { + return null; + } + + @Override + public String serialize(Dummy val) { + return null; + } } -} -public class ElideCustomSerdeRegistrationTest { @Test public void testRegisterCustomSerde() { HashMapDataStore wrapped = new HashMapDataStore(Dummy.class.getPackage()); diff --git a/elide-core/src/test/java/com/yahoo/elide/core/EntityDictionaryTest.java b/elide-core/src/test/java/com/yahoo/elide/core/EntityDictionaryTest.java index b767465c88..c55785626a 100644 --- a/elide-core/src/test/java/com/yahoo/elide/core/EntityDictionaryTest.java +++ b/elide-core/src/test/java/com/yahoo/elide/core/EntityDictionaryTest.java @@ -105,17 +105,17 @@ public void testFindCheckByExpression() { assertEquals("Prefab.Common.UpdateOnCreate", getCheckIdentifier(UpdateOnCreate.class)); } - @Test - public void testCheckScan() { - - @SecurityCheck("User is Admin") - class Foo extends UserCheck { + @SecurityCheck("User is Admin") + public class Foo extends UserCheck { - @Override - public boolean ok(com.yahoo.elide.security.User user) { - return false; - } + @Override + public boolean ok(com.yahoo.elide.security.User user) { + return false; } + } + + @Test + public void testCheckScan() { EntityDictionary testDictionary = new EntityDictionary(new HashMap<>()); testDictionary.scanForSecurityChecks(); diff --git a/elide-core/src/test/java/com/yahoo/elide/core/utils/ClassScannerTest.java b/elide-core/src/test/java/com/yahoo/elide/core/utils/ClassScannerTest.java index 132931b5dd..5b6eb5a866 100644 --- a/elide-core/src/test/java/com/yahoo/elide/core/utils/ClassScannerTest.java +++ b/elide-core/src/test/java/com/yahoo/elide/core/utils/ClassScannerTest.java @@ -27,18 +27,14 @@ public void testGetAllClasses() { @Test public void testGetAnnotatedClasses() { Set> classes = ClassScanner.getAnnotatedClasses("example", ReadPermission.class); - assertEquals(6, classes.size()); - for (Class cls : classes) { - assertTrue(cls.isAnnotationPresent(ReadPermission.class)); - } + assertEquals(6, classes.size(), "Actual: " + classes); + classes.forEach(cls -> assertTrue(cls.isAnnotationPresent(ReadPermission.class))); } @Test public void testGetAllAnnotatedClasses() { Set> classes = ClassScanner.getAnnotatedClasses(ReadPermission.class); - assertEquals(20, classes.size()); - for (Class cls : classes) { - assertTrue(cls.isAnnotationPresent(ReadPermission.class)); - } + assertEquals(12, classes.size(), "Actual: " + classes); + classes.forEach(cls -> assertTrue(cls.isAnnotationPresent(ReadPermission.class))); } }