From c25c07dfdddb7060f04ff32945c287f9b49b29fd Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Wed, 5 Aug 2015 09:53:51 +0200 Subject: [PATCH] Remove dead code ConfigurationPropertiesReportEndpoint parses the meta-data to inspect entities that have potential cycles in them. The whole logic is based on the lookup of `META-INF/spring-configuration-metadata.json` files on the classpath. Unfortunately, the lookup instruction had a typo and did not retrieve any file. Surely that code was written with a clear intention in mind but it was effectively dead code outside tests so it has been removed. Closes gh-3310 --- ...ConfigurationPropertiesReportEndpoint.java | 182 +----------------- ...rtiesReportEndpointSerializationTests.java | 48 +---- .../src/test/resources/test-metadata.json | 12 -- 3 files changed, 4 insertions(+), 238 deletions(-) delete mode 100644 spring-boot-actuator/src/test/resources/test-metadata.json diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/ConfigurationPropertiesReportEndpoint.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/ConfigurationPropertiesReportEndpoint.java index 9dd1021ca6d..4233e1036c8 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/ConfigurationPropertiesReportEndpoint.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/ConfigurationPropertiesReportEndpoint.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2015 the original author or authors. + * Copyright 2012-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. @@ -16,33 +16,21 @@ package org.springframework.boot.actuate.endpoint; -import java.io.IOException; -import java.io.InputStream; import java.util.ArrayList; -import java.util.Collection; import java.util.Collections; import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Set; -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; -import org.springframework.beans.BeanWrapperImpl; import org.springframework.beans.BeansException; import org.springframework.boot.context.properties.ConfigurationBeanFactoryMetaData; import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; -import org.springframework.core.io.Resource; -import org.springframework.core.io.support.PathMatchingResourcePatternResolver; import org.springframework.util.ClassUtils; import org.springframework.util.StringUtils; -import com.fasterxml.jackson.core.JsonParseException; import com.fasterxml.jackson.databind.BeanDescription; -import com.fasterxml.jackson.databind.JsonMappingException; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.SerializationConfig; import com.fasterxml.jackson.databind.SerializationFeature; @@ -76,17 +64,10 @@ public class ConfigurationPropertiesReportEndpoint extends private static final String CGLIB_FILTER_ID = "cglibFilter"; - private static final Log logger = LogFactory - .getLog(ConfigurationPropertiesReportEndpoint.class); - private final Sanitizer sanitizer = new Sanitizer(); private ApplicationContext context; - private ConfigurationPropertiesMetaData metadata; - - private String metadataLocations = "classpath:*/META-INF/*spring-configuration-metadata.json"; - public ConfigurationPropertiesReportEndpoint() { super("configprops"); } @@ -100,14 +81,6 @@ public class ConfigurationPropertiesReportEndpoint extends this.sanitizer.setKeysToSanitize(keysToSanitize); } - /** - * Location path for JSON metadata about config properties. - * @param metadataLocations the metadataLocations to set - */ - public void setMetadataLocations(String metadataLocations) { - this.metadataLocations = metadataLocations; - } - @Override public Map invoke() { return extract(this.context); @@ -174,13 +147,10 @@ public class ConfigurationPropertiesReportEndpoint extends */ private Map safeSerialize(ObjectMapper mapper, Object bean, String prefix) { - if (this.metadata == null) { - this.metadata = new ConfigurationPropertiesMetaData(this.metadataLocations); - } try { @SuppressWarnings("unchecked") Map result = new HashMap(mapper.convertValue( - this.metadata.extractMap(bean, prefix), Map.class)); + bean, Map.class)); return result; } catch (Exception ex) { @@ -349,152 +319,4 @@ public class ConfigurationPropertiesReportEndpoint extends } } - /** - * Convenience class for grabbing and caching valid property names from - * /META-INF/spring-configuration-metadata.json so that metadata that is known to be - * valid can be used to pull the correct nested properties out of beans that might - * otherwise be tricky (contain cycles or other unserializable properties). - */ - protected static class ConfigurationPropertiesMetaData { - - private final String metadataLocations; - - private final Map> matched = new HashMap>(); - - private Set keys = null; - - public ConfigurationPropertiesMetaData(String metadataLocations) { - this.metadataLocations = metadataLocations; - } - - public boolean matches(String prefix) { - if (this.matched.containsKey(prefix)) { - return matchesInternal(prefix); - } - synchronized (this.matched) { - if (this.matched.containsKey(prefix)) { - return matchesInternal(prefix); - } - this.matched.put(prefix, findKeys(prefix)); - } - return matchesInternal(prefix); - } - - private boolean matchesInternal(String prefix) { - return this.matched.get(prefix) != null; - } - - private Set findKeys(String prefix) { - HashSet keys = new HashSet(); - for (String key : getKeys()) { - if (key.length() > prefix.length() - && key.startsWith(prefix) - && ".".equals(key.substring(prefix.length(), prefix.length() + 1))) { - keys.add(key.substring(prefix.length() + 1)); - } - } - return (keys.isEmpty() ? null : keys); - } - - private Set getKeys() { - if (this.keys != null) { - return this.keys; - } - this.keys = new HashSet(); - try { - ObjectMapper mapper = new ObjectMapper(); - Resource[] resources = new PathMatchingResourcePatternResolver() - .getResources(this.metadataLocations); - for (Resource resource : resources) { - addKeys(mapper, resource); - } - } - catch (IOException ex) { - logger.warn("Could not deserialize config properties metadata", ex); - } - return this.keys; - } - - @SuppressWarnings("unchecked") - private void addKeys(ObjectMapper mapper, Resource resource) throws IOException, - JsonParseException, JsonMappingException { - InputStream inputStream = resource.getInputStream(); - Map map = mapper.readValue(inputStream, Map.class); - Collection> metadata = (Collection>) map - .get("properties"); - for (Map value : metadata) { - try { - if (value.containsKey("type")) { - this.keys.add((String) value.get("name")); - } - } - catch (Exception ex) { - logger.warn("Could not parse config properties metadata", ex); - } - } - } - - public Object extractMap(Object bean, String prefix) { - if (!matches(prefix)) { - return bean; - } - Map map = new HashMap(); - for (String key : this.matched.get(prefix)) { - addProperty(bean, key, map); - } - return map; - } - - @SuppressWarnings("unchecked") - private void addProperty(Object bean, String key, Map map) { - String prefix = (key.contains(".") ? StringUtils.split(key, ".")[0] : key); - String suffix = (key.length() > prefix.length() ? key.substring(prefix - .length() + 1) : null); - String property = prefix; - if (bean instanceof Map) { - Map value = (Map) bean; - bean = new MapHolder(value); - property = "map[" + property + "]"; - } - BeanWrapperImpl wrapper = new BeanWrapperImpl(bean); - try { - Object value = wrapper.getPropertyValue(property); - if (value instanceof Map) { - Map nested = new HashMap(); - map.put(prefix, nested); - if (suffix != null) { - addProperty(value, suffix, nested); - } - } - else { - map.put(prefix, value); - } - } - catch (Exception ex) { - // Probably just lives on a different bean (it happens) - logger.debug("Could not parse config properties metadata '" + key + "': " - + ex.getMessage()); - } - } - - protected static class MapHolder { - - Map map = new HashMap(); - - public MapHolder(Map bean) { - this.map.putAll(bean); - } - - public Map getMap() { - return this.map; - } - - public void setMap(Map map) { - this.map = map; - } - - } - - } - } diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/ConfigurationPropertiesReportEndpointSerializationTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/ConfigurationPropertiesReportEndpointSerializationTests.java index a3ed21b83af..68f40d82c3d 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/ConfigurationPropertiesReportEndpointSerializationTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/ConfigurationPropertiesReportEndpointSerializationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2014 the original author or authors. + * Copyright 2012-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. @@ -114,26 +114,6 @@ public class ConfigurationPropertiesReportEndpointSerializationTests { assertEquals("Cannot serialize 'foo'", map.get("error")); } - @Test - @SuppressWarnings("unchecked") - public void testCycleAvoidedThroughManualMetadata() throws Exception { - this.context.register(MetadataCycleConfig.class); - EnvironmentTestUtils.addEnvironment(this.context, "bar.name:foo"); - this.context.refresh(); - ConfigurationPropertiesReportEndpoint report = this.context - .getBean(ConfigurationPropertiesReportEndpoint.class); - Map properties = report.invoke(); - Map nestedProperties = (Map) properties - .get("foo"); - assertNotNull(nestedProperties); - assertEquals("bar", nestedProperties.get("prefix")); - Map map = (Map) nestedProperties - .get("properties"); - assertNotNull(map); - assertEquals(1, map.size()); - assertEquals("foo", map.get("name")); - } - @Test @SuppressWarnings("unchecked") public void testMap() throws Exception { @@ -194,28 +174,6 @@ public class ConfigurationPropertiesReportEndpointSerializationTests { assertEquals("foo", ((List) map.get("list")).get(0)); } - @Test - @SuppressWarnings("unchecked") - public void testMapWithMetadata() throws Exception { - this.context.register(MetadataMapConfig.class); - EnvironmentTestUtils.addEnvironment(this.context, "spam.map.name:foo"); - this.context.refresh(); - ConfigurationPropertiesReportEndpoint report = this.context - .getBean(ConfigurationPropertiesReportEndpoint.class); - Map properties = report.invoke(); - Map nestedProperties = (Map) properties - .get("foo"); - assertNotNull(nestedProperties); - assertEquals("spam", nestedProperties.get("prefix")); - Map map = (Map) nestedProperties - .get("properties"); - assertNotNull(map); - System.err.println(nestedProperties); - // Only one property is mapped in metadata so the others are ignored - assertEquals(1, map.size()); - assertEquals("foo", ((Map) map.get("map")).get("name")); - } - @Test @SuppressWarnings("unchecked") public void testInetAddress() throws Exception { @@ -243,9 +201,7 @@ public class ConfigurationPropertiesReportEndpointSerializationTests { @Bean public ConfigurationPropertiesReportEndpoint endpoint() { - ConfigurationPropertiesReportEndpoint endpoint = new ConfigurationPropertiesReportEndpoint(); - endpoint.setMetadataLocations("classpath*:/test-metadata.json"); - return endpoint; + return new ConfigurationPropertiesReportEndpoint(); } } diff --git a/spring-boot-actuator/src/test/resources/test-metadata.json b/spring-boot-actuator/src/test/resources/test-metadata.json deleted file mode 100644 index 712014c6c68..00000000000 --- a/spring-boot-actuator/src/test/resources/test-metadata.json +++ /dev/null @@ -1,12 +0,0 @@ -{"properties": [ - { - "name": "bar.name", - "type": "java.util.Map", - "dataType": "java.lang.String" - }, - { - "name": "spam.map.name", - "type": "java.util.Map", - "dataType": "java.lang.String" - } ]} -