Allow mutating the list of children from a derived Element

Stephen Paul Weber created 2 years ago

Data and Field Element subclasses expect to be able to mutate the list of
children on the original Element that they are derived from using parse.  The
way the new setChildren worked resulted in not actually mutating the original.

Change this for a bindTo that makes sure all fields reference the original (and
hence any mutation on the new object mutates the original) and also a
replaceChildren that uses clear+addAll to ensure that the existing list is
mutated rather than a new list being used.

In practise, this affected room configuration and other data forms being
properly filled out.

Change summary

src/main/java/eu/siacs/conversations/xml/Element.java      | 14 ++++++++
src/main/java/eu/siacs/conversations/xmpp/forms/Data.java  |  5 +-
src/main/java/eu/siacs/conversations/xmpp/forms/Field.java |  9 ++--
3 files changed, 20 insertions(+), 8 deletions(-)

Detailed changes

src/main/java/eu/siacs/conversations/xml/Element.java

@@ -134,12 +134,26 @@ public class Element implements Node {
 		return this.children;
 	}
 
+	// Deprecated: you probably want bindTo or replaceChildren
 	public Element setChildren(List<Element> children) {
 		this.childNodes = new ArrayList(children);
 		this.children = children;
 		return this;
 	}
 
+	public void replaceChildren(List<Element> children) {
+		this.childNodes.clear();
+		this.childNodes.addAll(children);
+		this.children.clear();
+		this.children.addAll(children);
+	}
+
+	public void bindTo(Element original) {
+		this.attributes = original.attributes;
+		this.childNodes = original.childNodes;
+		this.children = original.children;
+	}
+
 	public final String getContent() {
 		return this.childNodes.stream().map(Node::getContent).filter(c -> c != null).collect(Collectors.joining());
 	}

src/main/java/eu/siacs/conversations/xmpp/forms/Data.java

@@ -77,13 +77,12 @@ public class Data extends Element {
 	}
 
 	private void removeUnnecessaryChildren() {
-		setChildren(getChildren().stream().filter(element -> element.getName().equals("field") || element.getName().equals("title")).collect(Collectors.toList()));
+		replaceChildren(getChildren().stream().filter(element -> element.getName().equals("field") || element.getName().equals("title")).collect(Collectors.toList()));
 	}
 
 	public static Data parse(Element element) {
 		Data data = new Data();
-		data.setAttributes(element.getAttributes());
-		data.setChildren(element.getChildren());
+		data.bindTo(element);
 		return data;
 	}
 

src/main/java/eu/siacs/conversations/xmpp/forms/Field.java

@@ -23,21 +23,20 @@ public class Field extends Element {
 	}
 
 	public void setValue(String value) {
-		setChildren(List.of(new Element("value").setContent(value)));
+		replaceChildren(List.of(new Element("value").setContent(value)));
 	}
 
 	public void setValues(Collection<String> values) {
-		setChildren(values.stream().map(val -> new Element("value").setContent(val)).collect(Collectors.toList()));
+		replaceChildren(values.stream().map(val -> new Element("value").setContent(val)).collect(Collectors.toList()));
 	}
 
 	public void removeNonValueChildren() {
-		setChildren(getChildren().stream().filter(element -> element.getName().equals("value")).collect(Collectors.toList()));
+		replaceChildren(getChildren().stream().filter(element -> element.getName().equals("value")).collect(Collectors.toList()));
 	}
 
 	public static Field parse(Element element) {
 		Field field = new Field();
-		field.setAttributes(element.getAttributes());
-		field.setChildren(element.getChildren());
+		field.bindTo(element);
 		return field;
 	}