Skip to content

Add merge support to nested collection properties #481

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Add merge support to nested collection properties #481

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6a4ebe4
Test case for SPR-9552
cbaldwin74 Mar 1, 2014
bec2fd1
Add merge support to nested collection properties
cbaldwin74 Mar 3, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 64 additions & 3 deletions spring-beans/src/main/java/org/springframework/beans/MutablePropertyValues.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2013 the original author or authors.
* Copyright 2002-2014 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.
Expand All @@ -20,9 +20,12 @@
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.ListIterator;
import java.util.Map;
import java.util.Set;

import org.springframework.beans.factory.config.BeanDefinition;
import org.springframework.beans.factory.config.BeanDefinitionHolder;
import org.springframework.util.StringUtils;

/**
Expand Down Expand Up @@ -171,11 +174,69 @@ public MutablePropertyValues addPropertyValue(PropertyValue pv) {
setPropertyValueAt(pv, i);
return this;
}
else if (addNestedPropertyValue(pv, currentPv)) {
return this;
}
}
this.propertyValueList.add(pv);
return this;
}

/**
* Add a PropertyValue object, replacing any existing one for the
* corresponding property or getting merged with it (if applicable),
* after drilling down through any chains of beans to locate nested
* properties.
* @param pv PropertyValue object to add
* @param currentPv
* @return true if a matching property was located and merged if applicable, otherwise false
*/
private boolean addNestedPropertyValue(PropertyValue pv, PropertyValue currentPv) {
String pvName = pv.getName();
// Check if the property has a nested path name and we are at the root of the chain
if (pvName.startsWith(currentPv.getName()) && pvName.contains(".")) {
String[] pvNameComponents = pvName.split("\\.");
// The properties of the current bean in the chain
MutablePropertyValues values = null;
// Counter to track which property we will set in the bean
int k = 0;
// Walk any chain of inner beans until we get to the end
for (int j = 0; j < pvNameComponents.length; j++) {
Object value = currentPv.getValue();
if (value instanceof BeanDefinitionHolder) {
BeanDefinition bd = ((BeanDefinitionHolder) value).getBeanDefinition();
values = bd.getPropertyValues();
if (values != null) {
ListIterator<PropertyValue> pIterator =
values.getPropertyValueList().listIterator();
k = 0;
boolean found = false;
while (pIterator.hasNext()) {
PropertyValue v = pIterator.next();
// Look for the next bean in the chain
if (v.getName().equals(pvNameComponents[j + 1])) {
currentPv = v;
found = true;
break;
}
k++;
}
if (!found) {
throw new FatalBeanException("Bad nested property " + pvName);
}
}
}
}
if (pvName.endsWith(currentPv.getName())) {
// Add the property to the innermost bean properties
pv = mergeIfRequired(pv, currentPv);
values.setPropertyValueAt(pv, k);;
return true;
}
}
return false;
}

/**
* Overloaded version of {@code addPropertyValue} that takes
* a property name and a property value.
Expand Down Expand Up @@ -220,10 +281,10 @@ private PropertyValue mergeIfRequired(PropertyValue newPv, PropertyValue current
Mergeable mergeable = (Mergeable) value;
if (mergeable.isMergeEnabled()) {
Object merged = mergeable.merge(currentPv.getValue());
return new PropertyValue(newPv.getName(), merged);
return new PropertyValue(currentPv.getName(), merged);
}
}
return newPv;
return new PropertyValue(currentPv.getName(), newPv.getValue());
}

/**
Expand Down
96 changes: 95 additions & 1 deletion ...java/org/springframework/beans/factory/xml/NestedBeansElementAttributeRecursionTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2013 the original author or authors.
* Copyright 2002-2014 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.
Expand All @@ -21,6 +21,9 @@
import static org.hamcrest.Matchers.hasItems;
import static org.junit.Assert.assertThat;

import java.util.List;
import java.util.Map;

import org.junit.Test;
import org.springframework.beans.factory.config.BeanDefinition;
import org.springframework.beans.factory.support.DefaultListableBeanFactory;
Expand Down Expand Up @@ -77,6 +80,97 @@ public void defaultMerge() {
hasItems("charlie", "delta", "echo", "foxtrot", "golf", "hotel"));
}

@Test
@SuppressWarnings("unchecked")
public void defaultMergeNestedPath() {
DefaultListableBeanFactory bf = new DefaultListableBeanFactory();
new XmlBeanDefinitionReader(bf).loadBeanDefinitions(
new ClassPathResource("NestedBeansElementAttributeRecursionTests-merge-nested-path-context.xml", this.getClass()));
FooBean foo = bf.getBean("childFoo", FooBean.class);

// should contain entries added by parent
assertThat("missing key 'kParent'", foo.getMapBean().getMap().containsKey("kParent"), is(true));
assertThat("missing value 'middleList'", foo.getMapBean().getList().contains("middleList"), is(true));
assertThat("too many values in list", foo.getMapBean().getList().size() == 1, is(true));
assertThat("missing value 'lParent'", foo.getMapBean().getListBean().getList().contains("lParent"), is(true));

// should also contain entries added by child
assertThat("missing key 'kChild'", foo.getMapBean().getMap().containsKey("kChild"), is(true));
assertThat("missing value 'lChild'", foo.getMapBean().getListBean().getList().contains("lChild"), is(true));
assertThat("missing value 'replace'", foo.getMapBean().getListBean().getReplaceList().contains("replace"), is(true));
assertThat("too many values in replaceList", foo.getMapBean().getListBean().getReplaceList().size() == 1, is(true));
}

static class FooBean {

private MapBean mapBean;

public MapBean getMapBean() {
return mapBean;
}

public void setMapBean(MapBean mapBean) {
this.mapBean = mapBean;
}
}

@SuppressWarnings("rawtypes")
static class MapBean {

private Map map;
private List list;
private ListBean listBean;

public void setMap(Map map) {
this.map = map;
}

public Map getMap() {
return map;
}

public void setList(List list) {
this.list = list;
}

public List getList()
{
return list;
}

public void setListBean(ListBean listBean) {
this.listBean = listBean;
}

public ListBean getListBean() {
return listBean;
}
}

@SuppressWarnings("rawtypes")
static class ListBean {

private List list;
private List replaceList;

public void setList(List list) {
this.list = list;
}

public List getList()
{
return list;
}

public void setReplaceList(List list) {
this.replaceList = list;
}

public List getReplaceList() {
return replaceList;
}
}

@Test
public void defaultAutowireCandidates() {
DefaultListableBeanFactory bf = new DefaultListableBeanFactory();
Expand Down
54 changes: 54 additions & 0 deletions ...beans/factory/xml/NestedBeansElementAttributeRecursionTests-merge-nested-path-context.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
<?xml version="1.0" encoding="UTF-8"?>
<beans xmlns="http://www.springframework.org/schema/beans"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans-3.1.xsd">

<bean id="parentFoo" class="org.springframework.beans.factory.xml.NestedBeansElementAttributeRecursionTests$FooBean">
<property name="mapBean">
<bean class="org.springframework.beans.factory.xml.NestedBeansElementAttributeRecursionTests$MapBean">
<property name="map">
<map>
<entry key="kParent" value="vParent"/>
</map>
</property>
<property name="list">
<list>
<value>middleList</value>
</list>
</property>
<property name="listBean">
<bean class="org.springframework.beans.factory.xml.NestedBeansElementAttributeRecursionTests$ListBean">
<property name="list">
<list>
<value>lParent</value>
</list>
</property>
<property name="replaceList">
<list>
<value>original</value>
</list>
</property>
</bean>
</property>
</bean>
</property>
</bean>

<bean id="childFoo" parent="parentFoo">
<property name="mapBean.map">
<map merge="true">
<entry key="kChild" value="vChild"/>
</map>
</property>
<property name="mapBean.listBean.list">
<list merge="true">
<value>lChild</value>
</list>
</property>
<property name="mapBean.listBean.replaceList">
<list>
<value>replace</value>
</list>
</property>
</bean>
</beans>