From 994d86992caf468402697b4bcf3685c24f28fed5 Mon Sep 17 00:00:00 2001 From: Philippe Marschall Date: Sun, 10 May 2015 11:15:42 +0200 Subject: [PATCH] Avoid eager formatting in pre-condition checks In general, the Spring Framework aims to construct error message strings only if an actual error has occurred. This seems to be the common pattern in the codebase and saves both CPU and memory. However, there are some places where eager error message formatting occurs unnecessarily. This commit addresses this issue in the following classes: AdviceModeImportSelector, AnnotationAttributes, and ReadOnlySystemAttributesMap. The change in ReadOnlySystemAttributesMap also avoids a potential NullPointerException. Issue: SPR-13007 --- .../context/annotation/AdviceModeImportSelector.java | 9 ++++++--- .../core/annotation/AnnotationAttributes.java | 10 ++++++---- .../core/env/ReadOnlySystemAttributesMap.java | 11 ++++++----- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/spring-context/src/main/java/org/springframework/context/annotation/AdviceModeImportSelector.java b/spring-context/src/main/java/org/springframework/context/annotation/AdviceModeImportSelector.java index 7da0201809..14f9f1bfa2 100644 --- a/spring-context/src/main/java/org/springframework/context/annotation/AdviceModeImportSelector.java +++ b/spring-context/src/main/java/org/springframework/context/annotation/AdviceModeImportSelector.java @@ -21,7 +21,6 @@ import java.lang.annotation.Annotation; import org.springframework.core.GenericTypeResolver; import org.springframework.core.annotation.AnnotationAttributes; import org.springframework.core.type.AnnotationMetadata; -import org.springframework.util.Assert; /** * Convenient base class for {@link ImportSelector} implementations that select imports @@ -62,13 +61,17 @@ public abstract class AdviceModeImportSelector implements public final String[] selectImports(AnnotationMetadata importingClassMetadata) { Class annoType = GenericTypeResolver.resolveTypeArgument(getClass(), AdviceModeImportSelector.class); AnnotationAttributes attributes = AnnotationConfigUtils.attributesFor(importingClassMetadata, annoType); - Assert.notNull(attributes, String.format( + if (attributes == null) { + throw new IllegalArgumentException(String.format( "@%s is not present on importing class '%s' as expected", annoType.getSimpleName(), importingClassMetadata.getClassName())); + } AdviceMode adviceMode = attributes.getEnum(this.getAdviceModeAttributeName()); String[] imports = selectImports(adviceMode); - Assert.notNull(imports, String.format("Unknown AdviceMode: '%s'", adviceMode)); + if (imports == null) { + throw new IllegalArgumentException(String.format("Unknown AdviceMode: '%s'", adviceMode)); + } return imports; } diff --git a/spring-core/src/main/java/org/springframework/core/annotation/AnnotationAttributes.java b/spring-core/src/main/java/org/springframework/core/annotation/AnnotationAttributes.java index 090621d803..a8bf6c21f5 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/AnnotationAttributes.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/AnnotationAttributes.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2015 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -105,7 +105,9 @@ public class AnnotationAttributes extends LinkedHashMap { private T doGet(String attributeName, Class expectedType) { Assert.hasText(attributeName, "attributeName must not be null or empty"); Object value = get(attributeName); - Assert.notNull(value, String.format("Attribute '%s' not found", attributeName)); + if (value == null) { + throw new IllegalArgumentException(String.format("Attribute '%s' not found", attributeName)); + } if (!expectedType.isInstance(value)) { if (expectedType.isArray() && expectedType.getComponentType().isInstance(value)) { Object arrayValue = Array.newInstance(expectedType.getComponentType(), 1); @@ -114,8 +116,8 @@ public class AnnotationAttributes extends LinkedHashMap { } else { throw new IllegalArgumentException( - String.format("Attribute '%s' is of type [%s], but [%s] was expected. Cause: ", - attributeName, value.getClass().getSimpleName(), expectedType.getSimpleName())); + String.format("Attribute '%s' is of type [%s], but [%s] was expected.", + attributeName, value.getClass().getName(), expectedType.getName())); } } return (T) value; diff --git a/spring-core/src/main/java/org/springframework/core/env/ReadOnlySystemAttributesMap.java b/spring-core/src/main/java/org/springframework/core/env/ReadOnlySystemAttributesMap.java index b5826c51c7..9cca8afdfa 100644 --- a/spring-core/src/main/java/org/springframework/core/env/ReadOnlySystemAttributesMap.java +++ b/spring-core/src/main/java/org/springframework/core/env/ReadOnlySystemAttributesMap.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2013 the original author or authors. + * Copyright 2002-2015 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -21,8 +21,6 @@ import java.util.Collections; import java.util.Map; import java.util.Set; -import org.springframework.util.Assert; - /** * Read-only {@code Map} implementation that is backed by system * properties or environment variables. @@ -50,8 +48,11 @@ abstract class ReadOnlySystemAttributesMap implements Map { */ @Override public String get(Object key) { - Assert.isInstanceOf(String.class, key, - String.format("Expected key [%s] to be of type String, got %s", key, key.getClass().getName())); + if (!(key instanceof String)) { + throw new IllegalArgumentException( + "Key of type [" + (key != null ? key.getClass().getName() : "null") + + "] must be an instance of java.lang.String."); + } return this.getSystemAttribute((String) key); }