From 9343844f157670c0998d0ab356c72fb8be6ede66 Mon Sep 17 00:00:00 2001 From: Olivier Lamy Date: Mon, 14 Dec 2020 08:46:35 +1000 Subject: [PATCH] Jetty 9.4.x spotbug issue map iteration using entrySet(), diamond list creation (#5804) * fix some spotbug performance map iterations Signed-off-by: olivier lamy * cannot use computeIfAbsent because it is a PathMap Signed-off-by: olivier lamy --- .../security/ConstraintSecurityHandler.java | 33 ++++++++-------- .../org/eclipse/jetty/start/Licensing.java | 9 ++--- .../java/org/eclipse/jetty/util/Scanner.java | 13 ++++--- .../jetty/webapp/FragmentConfiguration.java | 11 +----- .../org/eclipse/jetty/webapp/MetaData.java | 38 +++++++++---------- .../org/eclipse/jetty/xml/XmlAppendable.java | 9 ++--- .../eclipse/jetty/xml/XmlConfiguration.java | 7 ++-- 7 files changed, 53 insertions(+), 67 deletions(-) diff --git a/jetty-security/src/main/java/org/eclipse/jetty/security/ConstraintSecurityHandler.java b/jetty-security/src/main/java/org/eclipse/jetty/security/ConstraintSecurityHandler.java index 97d0013e16f3..5396e0b363f9 100644 --- a/jetty-security/src/main/java/org/eclipse/jetty/security/ConstraintSecurityHandler.java +++ b/jetty-security/src/main/java/org/eclipse/jetty/security/ConstraintSecurityHandler.java @@ -163,7 +163,7 @@ public static List getConstraintMappingsForPath(String pathSp if (pathSpec == null || "".equals(pathSpec.trim()) || constraintMappings == null || constraintMappings.size() == 0) return Collections.emptyList(); - List mappings = new ArrayList(); + List mappings = new ArrayList<>(); for (ConstraintMapping mapping : constraintMappings) { if (pathSpec.equals(mapping.getPathSpec())) @@ -187,7 +187,7 @@ public static List removeConstraintMappingsForPath(String pat if (pathSpec == null || "".equals(pathSpec.trim()) || constraintMappings == null || constraintMappings.size() == 0) return Collections.emptyList(); - List mappings = new ArrayList(); + List mappings = new ArrayList<>(); for (ConstraintMapping mapping : constraintMappings) { //Remove the matching mappings by only copying in non-matching mappings @@ -209,10 +209,10 @@ public static List removeConstraintMappingsForPath(String pat */ public static List createConstraintsWithMappingsForPath(String name, String pathSpec, ServletSecurityElement securityElement) { - List mappings = new ArrayList(); + List mappings = new ArrayList<>(); //Create a constraint that will describe the default case (ie if not overridden by specific HttpMethodConstraints) - Constraint httpConstraint = null; + Constraint httpConstraint; ConstraintMapping httpConstraintMapping = null; if (securityElement.getEmptyRoleSemantic() != EmptyRoleSemantic.PERMIT || @@ -229,7 +229,7 @@ public static List createConstraintsWithMappingsForPath(Strin } //See Spec 13.4.1.2 p127 - List methodOmissions = new ArrayList(); + List methodOmissions = new ArrayList<>(); //make constraint mappings for this url for each of the HttpMethodConstraintElements Collection methodConstraintElements = securityElement.getHttpMethodConstraints(); @@ -254,7 +254,7 @@ public static List createConstraintsWithMappingsForPath(Strin //See spec 13.4.1.2 p127 - add an omission for every method name to the default constraint //UNLESS the default constraint contains all default values. In that case, we won't add it. See Servlet Spec 3.1 pg 129 if (methodOmissions.size() > 0 && httpConstraintMapping != null) - httpConstraintMapping.setMethodOmissions(methodOmissions.toArray(new String[methodOmissions.size()])); + httpConstraintMapping.setMethodOmissions(methodOmissions.toArray(new String[0])); return mappings; } @@ -435,9 +435,10 @@ protected void doStop() throws Exception protected void processConstraintMapping(ConstraintMapping mapping) { Map mappings = _constraintMap.get(mapping.getPathSpec()); + if (mappings == null) { - mappings = new HashMap(); + mappings = new HashMap<>(); _constraintMap.put(mapping.getPathSpec(), mappings); } RoleInfo allMethodsRoleInfo = mappings.get(ALL_METHODS); @@ -588,7 +589,7 @@ protected RoleInfo prepareConstraintInfo(String pathInContext, Request request) if (roleInfo == null) { //No specific http-method names matched - List applicableConstraints = new ArrayList(); + List applicableConstraints = new ArrayList<>(); //Get info for constraint that matches all methods if it exists RoleInfo all = mappings.get(ALL_METHODS); @@ -778,11 +779,11 @@ public Set getPathsWithUncoveredHttpMethods() if (_denyUncoveredMethods) return Collections.emptySet(); - Set uncoveredPaths = new HashSet(); - - for (String path : _constraintMap.keySet()) + Set uncoveredPaths = new HashSet<>(); + for (Entry> entry : _constraintMap.entrySet()) { - Map methodMappings = _constraintMap.get(path); + Map methodMappings = entry.getValue(); + //Each key is either: // : an exact method name // : * which means that the constraint applies to every method @@ -790,7 +791,7 @@ public Set getPathsWithUncoveredHttpMethods() if (methodMappings.get(ALL_METHODS) != null) continue; //can't be any uncovered methods for this url path - boolean hasOmissions = omissionsExist(path, methodMappings); + boolean hasOmissions = omissionsExist(entry.getKey(), methodMappings); for (String method : methodMappings.keySet()) { @@ -800,7 +801,7 @@ public Set getPathsWithUncoveredHttpMethods() for (String m : omittedMethods) { if (!methodMappings.containsKey(m)) - uncoveredPaths.add(path); + uncoveredPaths.add(entry.getKey()); } } else @@ -808,7 +809,7 @@ public Set getPathsWithUncoveredHttpMethods() //an exact method name if (!hasOmissions) //an http-method does not have http-method-omission to cover the other method names - uncoveredPaths.add(path); + uncoveredPaths.add(entry.getKey()); } } } @@ -849,7 +850,7 @@ protected Set getOmittedMethods(String omission) return Collections.emptySet(); String[] strings = omission.split("\\."); - Set methods = new HashSet(); + Set methods = new HashSet<>(); for (int i = 0; i < strings.length - 1; i++) { methods.add(strings[i]); diff --git a/jetty-start/src/main/java/org/eclipse/jetty/start/Licensing.java b/jetty-start/src/main/java/org/eclipse/jetty/start/Licensing.java index 2839ebeea159..98f714c5f8df 100644 --- a/jetty-start/src/main/java/org/eclipse/jetty/start/Licensing.java +++ b/jetty-start/src/main/java/org/eclipse/jetty/start/Licensing.java @@ -71,13 +71,10 @@ public boolean acknowledgeLicenses() throws IOException System.err.printf(" + contains software not covered by the Eclipse Public License!%n"); System.err.printf(" + has not been audited for compliance with its license%n"); - for (String key : licenseMap.keySet()) + for (Map.Entry> entry : licenseMap.entrySet()) { - System.err.printf("%n Module: %s%n", key); - for (String line : licenseMap.get(key)) - { - System.err.printf(" + %s%n", line); - } + System.err.printf("%n Module: %s%n", entry.getKey()); + entry.getValue().forEach(line -> System.err.printf(" + %s%n", line)); } boolean licenseAck = false; diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/Scanner.java b/jetty-util/src/main/java/org/eclipse/jetty/util/Scanner.java index de627e8a0157..d33c2ed6aaa4 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/Scanner.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/Scanner.java @@ -181,8 +181,8 @@ public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) th if (rootIncludesExcludes != null && !rootIncludesExcludes.isEmpty()) { //accepted if not explicitly excluded and either is explicitly included or there are no explicit inclusions - Boolean result = rootIncludesExcludes.test(dir); - if (Boolean.TRUE == result) + boolean result = rootIncludesExcludes.test(dir); + if (result) accepted = true; } else @@ -215,8 +215,8 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IO if (rootIncludesExcludes != null && !rootIncludesExcludes.isEmpty()) { //accepted if not explicitly excluded and either is explicitly included or there are no explicit inclusions - Boolean result = rootIncludesExcludes.test(file); - if (Boolean.TRUE == result) + boolean result = rootIncludesExcludes.test(file); + if (result) accepted = true; } else if (_filter == null || _filter.accept(f.getParentFile(), f.getName())) @@ -663,11 +663,12 @@ public synchronized void scan() public synchronized void scanFiles() { _currentScan.clear(); - for (Path p : _scannables.keySet()) + for (Entry> entry : _scannables.entrySet()) { + Path p = entry.getKey(); try { - Files.walkFileTree(p, EnumSet.allOf(FileVisitOption.class),_scanDepth, new Visitor(p, _scannables.get(p), _currentScan)); + Files.walkFileTree(p, EnumSet.allOf(FileVisitOption.class),_scanDepth, new Visitor(p, entry.getValue(), _currentScan)); } catch (IOException e) { diff --git a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/FragmentConfiguration.java b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/FragmentConfiguration.java index a0a16b383fb6..55083b78c7bc 100644 --- a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/FragmentConfiguration.java +++ b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/FragmentConfiguration.java @@ -71,16 +71,9 @@ public void addWebFragments(final WebAppContext context, final MetaData metaData Map frags = (Map)context.getAttribute(FRAGMENT_RESOURCES); if (frags != null) { - for (Resource key : frags.keySet()) + for (Map.Entry entry : frags.entrySet()) { - if (key.isDirectory()) //tolerate the case where the library is a directory, not a jar. useful for OSGi for example - { - metaData.addFragment(key, frags.get(key)); - } - else //the standard case: a jar most likely inside WEB-INF/lib - { - metaData.addFragment(key, frags.get(key)); - } + metaData.addFragment(entry.getKey(), entry.getValue()); } } } diff --git a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/MetaData.java b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/MetaData.java index bdac19c2bb49..3ae900f4a356 100644 --- a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/MetaData.java +++ b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/MetaData.java @@ -44,20 +44,20 @@ public class MetaData public static final String ORDERED_LIBS = "javax.servlet.context.orderedLibs"; public static final Resource NON_FRAG_RESOURCE = EmptyResource.INSTANCE; - protected Map _origins = new HashMap(); + protected Map _origins = new HashMap<>(); protected WebDescriptor _webDefaultsRoot; protected WebDescriptor _webXmlRoot; - protected final List _webOverrideRoots = new ArrayList(); + protected final List _webOverrideRoots = new ArrayList<>(); protected boolean _metaDataComplete; - protected final List _descriptorProcessors = new ArrayList(); - protected final List _webFragmentRoots = new ArrayList(); - protected final Map _webFragmentNameMap = new HashMap(); - protected final Map _webFragmentResourceMap = new HashMap(); - protected final Map> _annotations = new HashMap>(); - protected final List _webInfClasses = new ArrayList(); - protected final List _webInfJars = new ArrayList(); - protected final List _orderedContainerResources = new ArrayList(); - protected final List _orderedWebInfResources = new ArrayList(); + protected final List _descriptorProcessors = new ArrayList<>(); + protected final List _webFragmentRoots = new ArrayList<>(); + protected final Map _webFragmentNameMap = new HashMap<>(); + protected final Map _webFragmentResourceMap = new HashMap<>(); + protected final Map> _annotations = new HashMap<>(); + protected final List _webInfClasses = new ArrayList<>(); + protected final List _webInfJars = new ArrayList<>(); + protected final List _orderedContainerResources = new ArrayList<>(); + protected final List _orderedWebInfResources = new ArrayList<>(); protected Ordering _ordering;//can be set to RelativeOrdering by web-default.xml, web.xml, web-override.xml protected boolean _allowDuplicateFragmentNames = false; protected boolean _validateXml = false; @@ -333,12 +333,8 @@ public synchronized void addDiscoveredAnnotation(DiscoveredAnnotation annotation if (resource == null || !_webInfJars.contains(resource)) resource = EmptyResource.INSTANCE; - List list = _annotations.get(resource); - if (list == null) - { - list = new ArrayList(); - _annotations.put(resource, list); - } + List list = + _annotations.computeIfAbsent(resource, k -> new ArrayList<>()); list.add(annotation); } @@ -378,7 +374,7 @@ public void resolve(WebAppContext context) if (getOrdering() != null) { orderedWebInfJars = getOrderedWebInfJars(); - List orderedLibs = new ArrayList(); + List orderedLibs = new ArrayList<>(); for (Resource webInfJar : orderedWebInfJars) { //get just the name of the jar file @@ -545,10 +541,10 @@ public Resource getJarForFragment(String name) return null; Resource jar = null; - for (Resource r : _webFragmentResourceMap.keySet()) + for (Map.Entry entry : _webFragmentResourceMap.entrySet()) { - if (_webFragmentResourceMap.get(r).equals(f)) - jar = r; + if (entry.getValue().equals(f)) + jar = entry.getKey(); } return jar; } diff --git a/jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlAppendable.java b/jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlAppendable.java index 817c708a3024..71121ef10e03 100644 --- a/jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlAppendable.java +++ b/jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlAppendable.java @@ -59,7 +59,7 @@ public XmlAppendable(Appendable out, int indent, String encoding) throws IOExcep { _out = out; _indent = indent; - _out.append("\n"); + _out.append("\n"); } public XmlAppendable openTag(String tag, Map attributes) throws IOException @@ -153,11 +153,10 @@ public XmlAppendable closeTag() throws IOException private void attributes(Map attributes) throws IOException { - for (String k : attributes.keySet()) + for (Map.Entry entry : attributes.entrySet()) { - String v = attributes.get(k); - _out.append(' ').append(k).append("=\""); - content(v); + _out.append(' ').append(entry.getKey()).append("=\""); + content(entry.getValue()); _out.append('"'); } } diff --git a/jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlConfiguration.java b/jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlConfiguration.java index 62191d99cf78..31412518d0be 100644 --- a/jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlConfiguration.java +++ b/jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlConfiguration.java @@ -1884,10 +1884,9 @@ else if (arg.toLowerCase(Locale.ENGLISH).endsWith(".properties")) if (properties.size() > 0) { Map props = new HashMap<>(); - for (Object key : properties.keySet()) - { - props.put(key.toString(), String.valueOf(properties.get(key))); - } + properties.entrySet().stream() + .forEach(objectObjectEntry -> props.put(objectObjectEntry.getKey().toString(), + String.valueOf(objectObjectEntry.getValue()))); configuration.getProperties().putAll(props); }