From 7c2f403d55cb7588477891f015017eb5a81aea87 Mon Sep 17 00:00:00 2001 From: Grahame Grieve Date: Thu, 21 Sep 2023 12:40:30 +1000 Subject: [PATCH] fix problem with Element childMap getting out of sync --- .../org/hl7/fhir/r5/elementmodel/Element.java | 101 +++++-------- .../org/hl7/fhir/utilities/NamedItemList.java | 140 ++++++++++++++++++ 2 files changed, 174 insertions(+), 67 deletions(-) create mode 100644 org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/NamedItemList.java diff --git a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/elementmodel/Element.java b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/elementmodel/Element.java index 42077bd47..5bf66f08b 100644 --- a/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/elementmodel/Element.java +++ b/org.hl7.fhir.r5/src/main/java/org/hl7/fhir/r5/elementmodel/Element.java @@ -54,6 +54,8 @@ import org.hl7.fhir.r5.terminologies.expansion.ValueSetExpansionOutcome; import org.hl7.fhir.r5.utils.ToolingExtensions; import org.hl7.fhir.utilities.ElementDecoration; import org.hl7.fhir.utilities.ElementDecoration.DecorationType; +import org.hl7.fhir.utilities.NamedItemList; +import org.hl7.fhir.utilities.NamedItemList.NamedItem; import org.hl7.fhir.utilities.SourceLocation; import org.hl7.fhir.utilities.Utilities; import org.hl7.fhir.utilities.validation.ValidationMessage; @@ -69,7 +71,7 @@ import org.hl7.fhir.utilities.xhtml.XhtmlNode; * @author Grahame Grieve * */ -public class Element extends Base { +public class Element extends Base implements NamedItem { private static final HashSet extensionList = new HashSet<>(Arrays.asList("extension", "modifierExtension")); public enum SpecialElement { @@ -109,7 +111,7 @@ public class Element extends Base { private String type; private String value; private int index = -1; - private List children; + private NamedItemList children; private Property property; private Property elementProperty; // this is used when special is set to true - it tracks the underlying element property which is used in a few places private int line; @@ -123,7 +125,6 @@ public class Element extends Base { private List messages; private boolean prohibited; private boolean required; - private Map> childMap; private int descendentCount; private int instanceId; private boolean isNull; @@ -149,7 +150,7 @@ public class Element extends Base { this.name = name; this.property = property; if (property.isResource()) { - children = new ArrayList<>(); + children = new NamedItemList<>(); } } @@ -190,9 +191,9 @@ public class Element extends Base { return !(children == null || children.isEmpty()); } - public List getChildren() { + public NamedItemList getChildren() { if (children == null) - children = new ArrayList(); + children = new NamedItemList(); return children; } @@ -233,21 +234,7 @@ public class Element extends Base { } public List getChildrenByName(String name) { - List res = new ArrayList(); - if (children.size() > 20) { - populateChildMap(); - List l = childMap.get(name); - if (l != null) { - res.addAll(l); - } - } else { - if (hasChildren()) { - for (Element child : children) - if (name.equals(child.getName())) - res.add(child); - } - } - return res; + return children.getByName(name); } public void numberChildren() { @@ -308,7 +295,7 @@ public class Element extends Base { public void setChildValue(String name, String value) { if (children == null) - children = new ArrayList(); + children = new NamedItemList(); for (Element child : children) { if (name.equals(child.getName())) { if (!child.isPrimitive()) @@ -316,7 +303,7 @@ public class Element extends Base { child.setValue(value); } } - childMap = null; + try { setProperty(name.hashCode(), name, new StringType(value)); } catch (FHIRException e) { @@ -327,8 +314,7 @@ public class Element extends Base { public List getChildren(String name) { List res = new ArrayList(); if (children.size() > 20) { - populateChildMap(); - List l = childMap.get(name); + List l = children.getByName(name); if (l != null) { res.addAll(l); } @@ -367,8 +353,7 @@ public class Element extends Base { List result = new ArrayList(); if (children != null) { if (children.size() > 20) { - populateChildMap(); - List l = childMap.get(name); + List l = children.getByName(name); if (l != null) { result.addAll(l); } @@ -389,27 +374,6 @@ public class Element extends Base { return result.toArray(new Base[result.size()]); } - private void populateChildMap() { - if (childMap == null) { - childMap = new HashMap<>(); - for (Element child : children) { - String n; - if (child.getProperty().getName().endsWith("[x]")) { - n = child.getProperty().getName(); - n = n.substring(0, n.length()-3); - } else { - n = child.getName(); - } - List l = childMap.get(n); - if (l == null) { - l = new ArrayList(); - childMap.put(n,l); - } - l.add(child); - } - } - } - @Override protected void listChildren(List childProps) { if (children != null) { @@ -446,9 +410,8 @@ public class Element extends Base { throw new FHIRException("Cannot set property "+name+" on "+this.name+" - value is not a primitive type ("+value.fhirType()+") or an ElementModel type"); } - childMap = null; if (children == null) - children = new ArrayList(); + children = new NamedItemList(); Element childForValue = null; // look through existing children @@ -505,7 +468,7 @@ public class Element extends Base { } if (ve.children != null) { if (childForValue.children == null) - childForValue.children = new ArrayList(); + childForValue.children = new NamedItemList(); else childForValue.children.clear(); childForValue.children.addAll(ve.children); @@ -533,7 +496,7 @@ public class Element extends Base { public Element makeElement(String name) throws FHIRException { if (children == null) - children = new ArrayList(); + children = new NamedItemList(); // look through existing children for (Element child : children) { @@ -572,7 +535,7 @@ public class Element extends Base { public Element forceElement(String name) throws FHIRException { if (children == null) - children = new ArrayList(); + children = new NamedItemList(); // look through existing children for (Element child : children) { @@ -669,7 +632,6 @@ public class Element extends Base { for (Element e : children) { e.clearDecorations(); } - childMap = null; } public void markValidation(StructureDefinition profile, ElementDefinition definition) { @@ -694,9 +656,8 @@ public class Element extends Base { if (children == null) return null; if (children.size() > 20) { - populateChildMap(); - List l = childMap.get(name); - if (l == null) { + List l = children.getByName(name); + if (l == null || l.size() == 0) { // try the other way (in case of complicated naming rules) } else if (l.size() > 1) { throw new Error("Attempt to read a single element when there is more than one present ("+name+")"); @@ -724,8 +685,7 @@ public class Element extends Base { public void getNamedChildren(String name, List list) { if (children != null) if (children.size() > 20) { - populateChildMap(); - List l = childMap.get(name); + List l = children.getByName(name); if (l != null) { list.addAll(l); } @@ -918,8 +878,7 @@ public class Element extends Base { remove.add(child); } children.removeAll(remove); - Collections.sort(children, new ElementSortComparator(this, this.property)); - childMap = null; + children.sort(new ElementSortComparator(this, this.property)); } } @@ -1125,7 +1084,6 @@ public class Element extends Base { public void clear() { comments = null; children.clear(); - childMap = null; property = null; elementProperty = null; xhtml = null; @@ -1157,7 +1115,6 @@ public class Element extends Base { public void removeChild(String name) { children.removeIf(n -> name.equals(n.getName())); - childMap = null; } public boolean isProhibited() { @@ -1331,7 +1288,7 @@ public class Element extends Base { public Element addElement(String name) { if (children == null) - children = new ArrayList(); + children = new NamedItemList(); for (Property p : property.getChildProperties(this.name, type)) { if (p.getName().equals(name)) { @@ -1367,13 +1324,13 @@ public class Element extends Base { } dest.value = value; if (children != null) { - dest.children = new ArrayList<>(); + dest.children = new NamedItemList<>(); for (Element child : children) { dest.children.add((Element) child.copy()); } } else { dest.children = null; - } + } dest.line = line; dest.col = col; dest.xhtml = xhtml; @@ -1383,7 +1340,6 @@ public class Element extends Base { dest.messages = null; dest.prohibited = prohibited; dest.required = required; - dest.childMap = null; dest.descendentCount = descendentCount; dest.instanceId = instanceId; dest.isNull = isNull; @@ -1499,4 +1455,15 @@ public class Element extends Base { ext.addElement("url").setValue("value"); ext.addElement("valueString").setValue(translation); } + + @Override + public String getListName() { + if (getProperty().getName().endsWith("[x]")) { + String n = getProperty().getName(); + return n.substring(0, n.length()-3); + } else { + return getName(); + } + } + } \ No newline at end of file diff --git a/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/NamedItemList.java b/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/NamedItemList.java new file mode 100644 index 000000000..8d005bd22 --- /dev/null +++ b/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/NamedItemList.java @@ -0,0 +1,140 @@ +package org.hl7.fhir.utilities; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.Comparator; +import java.util.HashMap; +import java.util.Iterator; +import java.util.List; +import java.util.Map; + +public class NamedItemList implements Collection { + public interface NamedItem { + public String getListName(); + } + + + private static final int SIZE_CUTOFF_MAP = 10; + + + private List list = new ArrayList<>(); + private Map> map = null; + + @Override + public int size() { + return list.size(); + } + + @Override + public boolean isEmpty() { + return list.isEmpty(); + } + + @Override + public boolean contains(Object o) { + return list.contains(o); + } + + @Override + public Iterator iterator() { + return list.iterator(); + } + + @Override + public Object[] toArray() { + return list.toArray(); + } + + @Override + public T[] toArray(T[] a) { + return list.toArray(a); + } + + @Override + public boolean add(T e) { + map = null; + return list.add(e); + } + public void add(int index, T e) { + list.add(index, e); + map = null; + } + + @Override + public boolean remove(Object o) { + map = null; + return list.remove(o); + } + + @Override + public boolean containsAll(Collection c) { + return list.containsAll(c); + } + + @Override + public boolean addAll(Collection c) { + map = null; + return list.addAll(c); + } + + @Override + public boolean removeAll(Collection c) { + map = null; + return list.removeAll(c); + } + + @Override + public boolean retainAll(Collection c) { + map = null; + return list.retainAll(c); + } + + @Override + public void clear() { + list.clear(); + map = null; + } + + public List getByName(String name) { + List res = new ArrayList<>(); + if (size() > SIZE_CUTOFF_MAP) { + if (map == null) { + buildMap(); + } + List l = map.get(name); + if (l != null) { + res.addAll(l); + } + } else { + for (T child : list) { + if (name.equals(child.getListName())) { + res.add(child); + } + } + } + return res; + } + + public T get(int c) { + return list.get(c); + } + + private void buildMap() { + map = new HashMap<>(); + for (T child : list) { + String n = child.getListName(); + List l = map.get(n); + if (l == null) { + l = new ArrayList<>(); + map.put(n,l); + } + l.add(child); + } + } + + public void sort(Comparator sorter) { + Collections.sort(list, sorter); + } + +}