Browse Source

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
pull/763/merge
Philippe Marschall 10 years ago committed by Sam Brannen
parent
commit
994d86992c
  1. 9
      spring-context/src/main/java/org/springframework/context/annotation/AdviceModeImportSelector.java
  2. 10
      spring-core/src/main/java/org/springframework/core/annotation/AnnotationAttributes.java
  3. 11
      spring-core/src/main/java/org/springframework/core/env/ReadOnlySystemAttributesMap.java

9
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.GenericTypeResolver;
import org.springframework.core.annotation.AnnotationAttributes; import org.springframework.core.annotation.AnnotationAttributes;
import org.springframework.core.type.AnnotationMetadata; import org.springframework.core.type.AnnotationMetadata;
import org.springframework.util.Assert;
/** /**
* Convenient base class for {@link ImportSelector} implementations that select imports * Convenient base class for {@link ImportSelector} implementations that select imports
@ -62,13 +61,17 @@ public abstract class AdviceModeImportSelector<A extends Annotation> implements
public final String[] selectImports(AnnotationMetadata importingClassMetadata) { public final String[] selectImports(AnnotationMetadata importingClassMetadata) {
Class<?> annoType = GenericTypeResolver.resolveTypeArgument(getClass(), AdviceModeImportSelector.class); Class<?> annoType = GenericTypeResolver.resolveTypeArgument(getClass(), AdviceModeImportSelector.class);
AnnotationAttributes attributes = AnnotationConfigUtils.attributesFor(importingClassMetadata, annoType); 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", "@%s is not present on importing class '%s' as expected",
annoType.getSimpleName(), importingClassMetadata.getClassName())); annoType.getSimpleName(), importingClassMetadata.getClassName()));
}
AdviceMode adviceMode = attributes.getEnum(this.getAdviceModeAttributeName()); AdviceMode adviceMode = attributes.getEnum(this.getAdviceModeAttributeName());
String[] imports = selectImports(adviceMode); 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; return imports;
} }

10
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"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -105,7 +105,9 @@ public class AnnotationAttributes extends LinkedHashMap<String, Object> {
private <T> T doGet(String attributeName, Class<T> expectedType) { private <T> T doGet(String attributeName, Class<T> expectedType) {
Assert.hasText(attributeName, "attributeName must not be null or empty"); Assert.hasText(attributeName, "attributeName must not be null or empty");
Object value = get(attributeName); 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.isInstance(value)) {
if (expectedType.isArray() && expectedType.getComponentType().isInstance(value)) { if (expectedType.isArray() && expectedType.getComponentType().isInstance(value)) {
Object arrayValue = Array.newInstance(expectedType.getComponentType(), 1); Object arrayValue = Array.newInstance(expectedType.getComponentType(), 1);
@ -114,8 +116,8 @@ public class AnnotationAttributes extends LinkedHashMap<String, Object> {
} }
else { else {
throw new IllegalArgumentException( throw new IllegalArgumentException(
String.format("Attribute '%s' is of type [%s], but [%s] was expected. Cause: ", String.format("Attribute '%s' is of type [%s], but [%s] was expected.",
attributeName, value.getClass().getSimpleName(), expectedType.getSimpleName())); attributeName, value.getClass().getName(), expectedType.getName()));
} }
} }
return (T) value; return (T) value;

11
spring-core/src/main/java/org/springframework/core/env/ReadOnlySystemAttributesMap.java vendored

@ -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"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with 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.Map;
import java.util.Set; import java.util.Set;
import org.springframework.util.Assert;
/** /**
* Read-only {@code Map<String, String>} implementation that is backed by system * Read-only {@code Map<String, String>} implementation that is backed by system
* properties or environment variables. * properties or environment variables.
@ -50,8 +48,11 @@ abstract class ReadOnlySystemAttributesMap implements Map<String, String> {
*/ */
@Override @Override
public String get(Object key) { public String get(Object key) {
Assert.isInstanceOf(String.class, key, if (!(key instanceof String)) {
String.format("Expected key [%s] to be of type String, got %s", key, key.getClass().getName())); 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); return this.getSystemAttribute((String) key);
} }

Loading…
Cancel
Save