Minor class path scanning optimization (#642) (#647)

Johannes Eriksson · mshabarov · web-flow · commit 2ca873524f53 · 2020-07-23T14:22:03.000+03:00
Extend the default class scanning exclusion list to JDK and IDE classes.
Use prefix tree for faster lookups in long exclusion/inclusion lists.

Co-authored-by: Mikhail Shabarov &lt;61410877+mshabarov@users.noreply.github.com&gt;
diff --git a/vaadin-spring/src/main/java/com/vaadin/flow/spring/PrefixTree.java b/vaadin-spring/src/main/java/com/vaadin/flow/spring/PrefixTree.java
@@ -0,0 +1,72 @@
+/*
+ * Copyright 2000-2020 Vaadin Ltd.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.vaadin.flow.spring;
+
+import java.io.Serializable;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * Quick prefix lookup for package inclusion and exclusion lists.
+ */
+class PrefixTree implements Serializable {
+
+    private final Node root;
+
+    PrefixTree(Collection<String> prefixes) {
+        root = new Node();
+        root.terminal = false;
+        prefixes.forEach(this::addPrefix);
+    }
+
+    void addPrefix(String prefix) {
+        if (prefix.isEmpty()) {
+            throw new IllegalArgumentException("empty prefix");
+        }
+        root.addPrefix(prefix);
+    }
+
+    boolean hasPrefix(String s) {
+        Node node = root;
+        final int slen = s.length();
+        int sidx = 0;
+        while (node != null) {
+            if (node.terminal) {
+                return true;
+            } else if (sidx < slen) {
+                node = node.children.get(s.charAt(sidx++));
+            } else {
+                return false;
+            }
+        }
+        return false;
+    }
+
+    static class Node implements Serializable {
+        private final Map<Character, Node> children = new HashMap<>();
+        private boolean terminal = true;
+
+        void addPrefix(String prefix) {
+            terminal = false;
+            char ch = prefix.charAt(0);
+            children.putIfAbsent(ch, new Node());
+            if (prefix.length() > 1) {
+                children.get(ch).addPrefix(prefix.substring(1));
+            }
+        }
+    }
+}
diff --git a/vaadin-spring/src/main/java/com/vaadin/flow/spring/VaadinServletContextInitializer.java b/vaadin-spring/src/main/java/com/vaadin/flow/spring/VaadinServletContextInitializer.java
@@ -105,18 +105,38 @@ public class VaadinServletContextInitializer
     private ResourceLoader customLoader;
 
     /**
-     * packages that are white-listed (should be scanned) by default and can't
-     * be overriden by <code>addedWhiteListed</code>.
+     * Packages that should be excluded when scanning all packages.
      */
-    private static final List<String> DEFAULT_WHITE_LISTED = Stream
+    private static final List<String> DEFAULT_SCAN_NEVER = Stream.of("antlr",
+            "cglib", "ch/quos/logback", "commons-codec", "commons-fileupload",
+            "commons-io", "commons-logging", "com/fasterxml", "com/google",
+            "com/h2database", "com/helger", "com/vaadin/external/atmosphere",
+            "com/vaadin/webjar", "junit", "net/bytebuddy", "org/apache",
+            "org/aspectj", "org/bouncycastle", "org/dom4j", "org/easymock",
+            "org/hamcrest", "org/hibernate", "org/javassist", "org/jboss",
+            "org/jsoup", "org/seleniumhq", "org/slf4j", "org/atmosphere",
+            "org/springframework", "org/webjars/bowergithub", "org/yaml",
+
+            "java/", "javax/", "javafx/", "com/sun/", "oracle/deploy",
+            "oracle/javafx", "oracle/jrockit", "oracle/jvm", "oracle/net",
+            "oracle/nio", "oracle/tools", "oracle/util", "oracle/webservices",
+            "oracle/xmlns",
+
+            "com/intellij/", "org/jetbrains").collect(Collectors.toList());
+
+   /**
+     * Packages that should be scanned by default and can't be overriden by
+     * a custom list.
+     */
+    private static final List<String> DEFAULT_SCAN_ONLY = Stream
             .of(Component.class.getPackage().getName(),
                     Theme.class.getPackage().getName(), "com.vaadin.shrinkwrap")
             .collect(Collectors.toList());
 
     /**
-     * Packages whitelisted by the user
+     * Packages marked by the user to be scanned exclusively.
      */
-    private List<String> customWhitelist;
+    private final List<String> customScanOnly;
 
     /**
      * Class path scanner that reuses infrastructure from Spring while also
@@ -324,8 +344,8 @@ public void contextInitialized(ServletContextEvent event) {
             }
 
             Set<String> basePackages;
-            if (isWhitelistSet()) {
-                basePackages = new HashSet<>(getWhiteListPackages());
+            if (isScanOnlySet()) {
+                basePackages = new HashSet<>(getScanOnlyPackages());
             } else {
                 basePackages = Collections.singleton("");
             }
@@ -361,11 +381,11 @@ public void contextDestroyed(ServletContextEvent event) {
             // NO-OP
         }
 
-        private Collection<String> getWhiteListPackages() {
+        private Collection<String> getScanOnlyPackages() {
             HashSet<String> npmPackages = new HashSet<>(getDefaultPackages());
-            npmPackages.addAll(DEFAULT_WHITE_LISTED);
-            if (customWhitelist != null) {
-                npmPackages.addAll(customWhitelist);
+            npmPackages.addAll(DEFAULT_SCAN_ONLY);
+            if (customScanOnly != null) {
+                npmPackages.addAll(customScanOnly);
             }
             return npmPackages;
         }
@@ -386,8 +406,9 @@ private void collectDevModeTypes(
             }
         }
 
-        private boolean isWhitelistSet() {
-            return customWhitelist != null && !customWhitelist.isEmpty();
+        private boolean isScanOnlySet() {
+            return customScanOnly != null
+                    && !customScanOnly.isEmpty();
         }
     }
 
@@ -440,31 +461,30 @@ public void contextDestroyed(ServletContextEvent sce) {
      */
     public VaadinServletContextInitializer(ApplicationContext context) {
         appContext = context;
-        String blacklistProperty = appContext.getEnvironment()
+        String neverScanProperty = appContext.getEnvironment()
                 .getProperty("vaadin.blacklisted-packages");
-        List<String> blacklist;
-        if (blacklistProperty == null) {
-            blacklist = Collections.emptyList();
+        List<String> neverScan;
+        if (neverScanProperty == null) {
+            neverScan = Collections.emptyList();
         } else {
-            blacklist = Arrays.stream(blacklistProperty.split(","))
+            neverScan = Arrays.stream(neverScanProperty.split(","))
                     .map(String::trim).collect(Collectors.toList());
         }
 
-        String whitelistProperty = appContext.getEnvironment()
+        String onlyScanProperty = appContext.getEnvironment()
                 .getProperty("vaadin.whitelisted-packages");
-        if (whitelistProperty == null) {
-            customWhitelist = Collections.emptyList();
-            customLoader = new CustomResourceLoader(appContext, blacklist);
+        if (onlyScanProperty == null) {
+            customScanOnly = Collections.emptyList();
+            customLoader = new CustomResourceLoader(appContext, neverScan);
 
         } else {
-            customWhitelist = Arrays.stream(whitelistProperty.split(","))
-                    .map(whitelistedPackage -> whitelistedPackage
-                            .replace('/', '.').trim())
+            customScanOnly = Arrays.stream(onlyScanProperty.split(","))
+                    .map(onlyPackage -> onlyPackage.replace('/', '.').trim())
                     .collect(Collectors.toList());
             customLoader = appContext;
         }
 
-        if (!customWhitelist.isEmpty() && !blacklist.isEmpty()) {
+        if (!customScanOnly.isEmpty() && !neverScan.isEmpty()) {
             getLogger().warn(
                     "vaadin.blacklisted-packages is ignored because both vaadin.whitelisted-packages and vaadin.blacklisted-packages have been set.");
         }
@@ -604,34 +624,22 @@ private List<String> getDefaultPackages() {
      */
     private static class CustomResourceLoader
             extends PathMatchingResourcePatternResolver {
-        /**
-         * Blacklisted packages that shouldn't be scanned for when scanning all
-         * packages.
-         */
-        private List<String> blackListed = Stream.of("antlr", "cglib",
-                "ch/quos/logback", "commons-codec", "commons-fileupload",
-                "commons-io", "commons-logging", "com/fasterxml", "com/google",
-                "com/h2database", "com/helger",
-                "com/vaadin/external/atmosphere", "com/vaadin/webjar", "javax/",
-                "junit", "net/bytebuddy", "org/apache", "org/aspectj",
-                "org/bouncycastle", "org/dom4j", "org/easymock", "org/hamcrest",
-                "org/hibernate", "org/javassist", "org/jboss", "org/jsoup",
-                "org/seleniumhq", "org/slf4j", "org/atmosphere",
-                "org/springframework", "org/webjars/bowergithub", "org/yaml")
-                .collect(Collectors.toList());
-
-        private static List<String> defaultWhiteList = DEFAULT_WHITE_LISTED
-                .stream().map(packageName -> packageName.replace('.', '/'))
-                .collect(Collectors.toList());
+
+        private final PrefixTree scanNever = new PrefixTree(DEFAULT_SCAN_NEVER);
+
+        private final PrefixTree scanAlways = new PrefixTree(
+                DEFAULT_SCAN_ONLY.stream()
+                        .map(packageName -> packageName.replace('.', '/'))
+                        .collect(Collectors.toList()));
 
         public CustomResourceLoader(ResourceLoader resourceLoader,
-                List<String> addedBlacklist) {
+                List<String> addedScanNever) {
             super(resourceLoader);
 
-            Objects.requireNonNull(addedBlacklist,
-                    "addedBlacklist shouldn't be null!");
+            Objects.requireNonNull(addedScanNever,
+                    "addedScanNever shouldn't be null!");
 
-            blackListed.addAll(addedBlacklist);
+            addedScanNever.forEach(scanNever::addPrefix);
         }
 
         /**
@@ -699,11 +707,8 @@ private Resource[] collectResources(String locationPattern)
         }
 
         private boolean shouldPathBeScanned(String path) {
-            if (defaultWhiteList.stream().anyMatch(path::startsWith)) {
-                return true;
-            }
-
-            return !blackListed.stream().anyMatch(path::startsWith);
+            return scanAlways.hasPrefix(path)
+                    || !scanNever.hasPrefix(path);
         }
     }
 
diff --git a/vaadin-spring/src/test/java/com/vaadin/flow/spring/PrefixTreeTest.java b/vaadin-spring/src/test/java/com/vaadin/flow/spring/PrefixTreeTest.java
@@ -0,0 +1,51 @@
+/*
+ * Copyright 2000-2020 Vaadin Ltd.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.vaadin.flow.spring;
+
+import java.util.Arrays;
+import java.util.Collections;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class PrefixTreeTest {
+
+    @Test
+    public void hasPrefix_containsPrefix_returnsTrue() {
+        PrefixTree prefixTree = new PrefixTree(
+                Arrays.asList("com/sun", "antlr", "ch/quos/logback"));
+        Assert.assertTrue(prefixTree.hasPrefix("antlr"));
+        Assert.assertTrue(prefixTree.hasPrefix("com/sun/test"));
+        Assert.assertTrue(prefixTree.hasPrefix("com/sun"));
+    }
+
+    @Test
+    public void hasPrefix_doesNotContainPrefix_returnsFalse() {
+        PrefixTree prefixTree = new PrefixTree(
+                Arrays.asList("com/sun", "antlr", "ch/quos/logback"));
+        Assert.assertFalse(prefixTree.hasPrefix(""));
+        Assert.assertFalse(prefixTree.hasPrefix("a"));
+        Assert.assertFalse(prefixTree.hasPrefix("test"));
+        Assert.assertFalse(prefixTree.hasPrefix("com/su"));
+    }
+
+    @Test
+    public void hasPrefix_emptyTree_returnsFalse() {
+        PrefixTree prefixTree = new PrefixTree(Collections.emptyList());
+        Assert.assertFalse(prefixTree.hasPrefix("a"));
+    }
+
+}
