Skip to content

Commit

Permalink
Jetty 9.4.x spotbug issue map iteration using entrySet(), diamond lis…
Browse files Browse the repository at this point in the history 
…t creation (#5804)

* fix some spotbug performance map iterations

Signed-off-by: olivier lamy <oliver.lamy@gmail.com>

* cannot use computeIfAbsent because it is a PathMap

Signed-off-by: olivier lamy <oliver.lamy@gmail.com>
  • Loading branch information
@olamy
olamy committed Dec 13, 2020
1 parent 639cad6 commit 9343844
Show file tree
Hide file tree
Showing 7 changed files with 53 additions and 67 deletions.
33 changes: 17 additions & 16 deletions jetty-security/src/main/java/org/eclipse/jetty/security/ConstraintSecurityHandler.java
View file
Expand Up @@ -163,7 +163,7 @@ public static List<ConstraintMapping> getConstraintMappingsForPath(String pathSp
if (pathSpec == null || "".equals(pathSpec.trim()) || constraintMappings == null || constraintMappings.size() == 0)
return Collections.emptyList();

List<ConstraintMapping> mappings = new ArrayList<ConstraintMapping>();
List<ConstraintMapping> mappings = new ArrayList<>();
for (ConstraintMapping mapping : constraintMappings)
{
if (pathSpec.equals(mapping.getPathSpec()))
Expand All @@ -187,7 +187,7 @@ public static List<ConstraintMapping> removeConstraintMappingsForPath(String pat
if (pathSpec == null || "".equals(pathSpec.trim()) || constraintMappings == null || constraintMappings.size() == 0)
return Collections.emptyList();

List<ConstraintMapping> mappings = new ArrayList<ConstraintMapping>();
List<ConstraintMapping> mappings = new ArrayList<>();
for (ConstraintMapping mapping : constraintMappings)
{
//Remove the matching mappings by only copying in non-matching mappings
Expand All @@ -209,10 +209,10 @@ public static List<ConstraintMapping> removeConstraintMappingsForPath(String pat
*/
public static List<ConstraintMapping> createConstraintsWithMappingsForPath(String name, String pathSpec, ServletSecurityElement securityElement)
{
List<ConstraintMapping> mappings = new ArrayList<ConstraintMapping>();
List<ConstraintMapping> 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 ||
Expand All @@ -229,7 +229,7 @@ public static List<ConstraintMapping> createConstraintsWithMappingsForPath(Strin
}

//See Spec 13.4.1.2 p127
List<String> methodOmissions = new ArrayList<String>();
List<String> methodOmissions = new ArrayList<>();

//make constraint mappings for this url for each of the HttpMethodConstraintElements
Collection<HttpMethodConstraintElement> methodConstraintElements = securityElement.getHttpMethodConstraints();
Expand All @@ -254,7 +254,7 @@ public static List<ConstraintMapping> 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;
}
Expand Down Expand Up @@ -435,9 +435,10 @@ protected void doStop() throws Exception
protected void processConstraintMapping(ConstraintMapping mapping)
{
Map<String, RoleInfo> mappings = _constraintMap.get(mapping.getPathSpec());

if (mappings == null)
{
mappings = new HashMap<String, RoleInfo>();
mappings = new HashMap<>();
_constraintMap.put(mapping.getPathSpec(), mappings);
}
RoleInfo allMethodsRoleInfo = mappings.get(ALL_METHODS);
Expand Down Expand Up @@ -588,7 +589,7 @@ protected RoleInfo prepareConstraintInfo(String pathInContext, Request request)
if (roleInfo == null)
{
//No specific http-method names matched
List<RoleInfo> applicableConstraints = new ArrayList<RoleInfo>();
List<RoleInfo> applicableConstraints = new ArrayList<>();

//Get info for constraint that matches all methods if it exists
RoleInfo all = mappings.get(ALL_METHODS);
Expand Down Expand Up @@ -778,19 +779,19 @@ public Set<String> getPathsWithUncoveredHttpMethods()
if (_denyUncoveredMethods)
return Collections.emptySet();

Set<String> uncoveredPaths = new HashSet<String>();

for (String path : _constraintMap.keySet())
Set<String> uncoveredPaths = new HashSet<>();
for (Entry<String,Map<String, RoleInfo>> entry : _constraintMap.entrySet())
{
Map<String, RoleInfo> methodMappings = _constraintMap.get(path);
Map<String, RoleInfo> methodMappings = entry.getValue();

//Each key is either:
// : an exact method name
// : * which means that the constraint applies to every method
// : a name of the form <method>.<method>.<method>.omission, which means it applies to every method EXCEPT those named
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())
{
Expand All @@ -800,15 +801,15 @@ public Set<String> getPathsWithUncoveredHttpMethods()
for (String m : omittedMethods)
{
if (!methodMappings.containsKey(m))
uncoveredPaths.add(path);
uncoveredPaths.add(entry.getKey());
}
}
else
{
//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());
}
}
}
Expand Down Expand Up @@ -849,7 +850,7 @@ protected Set<String> getOmittedMethods(String omission)
return Collections.emptySet();

String[] strings = omission.split("\\.");
Set<String> methods = new HashSet<String>();
Set<String> methods = new HashSet<>();
for (int i = 0; i < strings.length - 1; i++)
{
methods.add(strings[i]);
Expand Down
9 changes: 3 additions & 6 deletions jetty-start/src/main/java/org/eclipse/jetty/start/Licensing.java
View file
Expand Up @@ -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<String, List<String>> 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;
Expand Down
13 changes: 7 additions & 6 deletions jetty-util/src/main/java/org/eclipse/jetty/util/Scanner.java
View file
Expand Up @@ -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
Expand Down Expand Up @@ -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()))
Expand Down Expand Up @@ -663,11 +663,12 @@ public synchronized void scan()
public synchronized void scanFiles()
{
_currentScan.clear();
for (Path p : _scannables.keySet())
for (Entry<Path, IncludeExcludeSet<PathMatcher, Path>> 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)
{
Expand Down
11 changes: 2 additions & 9 deletions jetty-webapp/src/main/java/org/eclipse/jetty/webapp/FragmentConfiguration.java
View file
Expand Up @@ -71,16 +71,9 @@ public void addWebFragments(final WebAppContext context, final MetaData metaData
Map<Resource, Resource> frags = (Map<Resource, Resource>)context.getAttribute(FRAGMENT_RESOURCES);
if (frags != null)
{
for (Resource key : frags.keySet())
for (Map.Entry<Resource, Resource> 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());
}
}
}
Expand Down
38 changes: 17 additions & 21 deletions jetty-webapp/src/main/java/org/eclipse/jetty/webapp/MetaData.java
View file
Expand Up @@ -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<String, OriginInfo> _origins = new HashMap<String, OriginInfo>();
protected Map<String, OriginInfo> _origins = new HashMap<>();
protected WebDescriptor _webDefaultsRoot;
protected WebDescriptor _webXmlRoot;
protected final List<WebDescriptor> _webOverrideRoots = new ArrayList<WebDescriptor>();
protected final List<WebDescriptor> _webOverrideRoots = new ArrayList<>();
protected boolean _metaDataComplete;
protected final List<DescriptorProcessor> _descriptorProcessors = new ArrayList<DescriptorProcessor>();
protected final List<FragmentDescriptor> _webFragmentRoots = new ArrayList<FragmentDescriptor>();
protected final Map<String, FragmentDescriptor> _webFragmentNameMap = new HashMap<String, FragmentDescriptor>();
protected final Map<Resource, FragmentDescriptor> _webFragmentResourceMap = new HashMap<Resource, FragmentDescriptor>();
protected final Map<Resource, List<DiscoveredAnnotation>> _annotations = new HashMap<Resource, List<DiscoveredAnnotation>>();
protected final List<Resource> _webInfClasses = new ArrayList<Resource>();
protected final List<Resource> _webInfJars = new ArrayList<Resource>();
protected final List<Resource> _orderedContainerResources = new ArrayList<Resource>();
protected final List<Resource> _orderedWebInfResources = new ArrayList<Resource>();
protected final List<DescriptorProcessor> _descriptorProcessors = new ArrayList<>();
protected final List<FragmentDescriptor> _webFragmentRoots = new ArrayList<>();
protected final Map<String, FragmentDescriptor> _webFragmentNameMap = new HashMap<>();
protected final Map<Resource, FragmentDescriptor> _webFragmentResourceMap = new HashMap<>();
protected final Map<Resource, List<DiscoveredAnnotation>> _annotations = new HashMap<>();
protected final List<Resource> _webInfClasses = new ArrayList<>();
protected final List<Resource> _webInfJars = new ArrayList<>();
protected final List<Resource> _orderedContainerResources = new ArrayList<>();
protected final List<Resource> _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;
Expand Down Expand Up @@ -333,12 +333,8 @@ public synchronized void addDiscoveredAnnotation(DiscoveredAnnotation annotation
if (resource == null || !_webInfJars.contains(resource))
resource = EmptyResource.INSTANCE;

List<DiscoveredAnnotation> list = _annotations.get(resource);
if (list == null)
{
list = new ArrayList<DiscoveredAnnotation>();
_annotations.put(resource, list);
}
List<DiscoveredAnnotation> list =
_annotations.computeIfAbsent(resource, k -> new ArrayList<>());
list.add(annotation);
}

Expand Down Expand Up @@ -378,7 +374,7 @@ public void resolve(WebAppContext context)
if (getOrdering() != null)
{
orderedWebInfJars = getOrderedWebInfJars();
List<String> orderedLibs = new ArrayList<String>();
List<String> orderedLibs = new ArrayList<>();
for (Resource webInfJar : orderedWebInfJars)
{
//get just the name of the jar file
Expand Down Expand Up @@ -545,10 +541,10 @@ public Resource getJarForFragment(String name)
return null;

Resource jar = null;
for (Resource r : _webFragmentResourceMap.keySet())
for (Map.Entry<Resource, FragmentDescriptor> entry : _webFragmentResourceMap.entrySet())
{
if (_webFragmentResourceMap.get(r).equals(f))
jar = r;
if (entry.getValue().equals(f))
jar = entry.getKey();
}
return jar;
}
Expand Down
9 changes: 4 additions & 5 deletions jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlAppendable.java
View file
Expand Up @@ -59,7 +59,7 @@ public XmlAppendable(Appendable out, int indent, String encoding) throws IOExcep
{
_out = out;
_indent = indent;
_out.append("<?xml version=\"1.0\" encoding=\"" + encoding + "\"?>\n");
_out.append("<?xml version=\"1.0\" encoding=\"").append(encoding).append("\"?>\n");
}

public XmlAppendable openTag(String tag, Map<String, String> attributes) throws IOException
Expand Down Expand Up @@ -153,11 +153,10 @@ public XmlAppendable closeTag() throws IOException

private void attributes(Map<String, String> attributes) throws IOException
{
for (String k : attributes.keySet())
for (Map.Entry<String, String> 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('"');
}
}
Expand Down
7 changes: 3 additions & 4 deletions jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlConfiguration.java
View file
Expand Up @@ -1884,10 +1884,9 @@ else if (arg.toLowerCase(Locale.ENGLISH).endsWith(".properties"))
if (properties.size() > 0)
{
Map<String, String> 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);
}

Expand Down

0 comments on commit 9343844

Please sign in to comment.