From f1327ce6cfaa4941c1b6cf2d1ad8122d02fff541 Mon Sep 17 00:00:00 2001 From: Cies Date: Thu, 28 Mar 2024 17:30:46 +0100 Subject: [PATCH] Improve performance, add test, small cleanups --- .../src/play/data/binding/ParamNode.java | 37 +++++++++---------- .../test/play/data/binding/BinderTest.java | 29 +++++++++------ 2 files changed, 36 insertions(+), 30 deletions(-) diff --git a/framework/src/play/data/binding/ParamNode.java b/framework/src/play/data/binding/ParamNode.java index ff088a9b..29e70d8c 100644 --- a/framework/src/play/data/binding/ParamNode.java +++ b/framework/src/play/data/binding/ParamNode.java @@ -3,6 +3,7 @@ import play.utils.Utils; import java.util.*; +import java.util.regex.Pattern; public class ParamNode { private final String name; @@ -10,14 +11,13 @@ public class ParamNode { private String[] values = null; private String originalKey; - // splits a string on one-ore-more instances of .[] - // this works so that all the following strings (param naming syntax) - // is resolved into the same structural hierarchy: + // Regex to split the key of an x-www-form-urlencoded key-value pair on one-ore-more instances of ".[]". + // Using this all the following strings (Play's naming syntax for keys) resolved into the same structural hierarchy: // a.b.c=12 // a[b].c=12 // a[b][c]=12 // a.b[c]=12 - private static final String keyPartDelimiterRegexpString = "[\\.\\[\\]]+"; + private static final Pattern keyPartDelimiterRegex = Pattern.compile("[.\\[\\]]+"); public ParamNode(String name) { this.name = name; @@ -36,15 +36,15 @@ public String getFirstValue(Class type) { return null; } - if (values.length>1 && String.class.equals(type)) { - // special handling for string - when multiple values, concatenate them with comma.. + if (values.length > 1 && String.class.equals(type)) { + // Special handling for strings: when multiple values are provided, concatenate them into one return Utils.join(values, ", "); } else { return values[0]; } } - public void addChild( ParamNode child) { + public void addChild(ParamNode child) { _children.put(child.name, child); } @@ -53,7 +53,7 @@ public ParamNode getChild(String name) { } public ParamNode getChild(String name, boolean returnEmptyChildIfNotFound) { - ParamNode child = getChild( name.split(keyPartDelimiterRegexpString)); + ParamNode child = getChild(keyPartDelimiterRegex.split(name)); if (child == null && returnEmptyChildIfNotFound) { child = new ParamNode(name); } @@ -72,7 +72,7 @@ public RemovedNode(ParamNode removedFrom, ParamNode removedNode) { /** * Removes a child from this node, but stores what is removed to list. - * The we can later call which will add it back again. + * Then we can later call which will add it back again. * This is a "hack" related to #1195 which makes it possible to reuse the RootParamsNode-structure * if you want to perform the bind-operation multiple times. * @@ -82,8 +82,8 @@ public RemovedNode(ParamNode removedFrom, ParamNode removedNode) { */ public boolean removeChild(String name, List removedNodesList) { ParamNode removedNode = _children.remove(name); - if ( removedNode != null) { - removedNodesList.add( new RemovedNode(this, removedNode)); + if (removedNode != null) { + removedNodesList.add(new RemovedNode(this, removedNode)); return true; } else { return false; @@ -91,15 +91,15 @@ public boolean removeChild(String name, List removedNodesList) { } public static void restoreRemovedChildren( List removedNodesList ) { - for ( RemovedNode rn : removedNodesList) { + for (RemovedNode rn : removedNodesList) { rn.removedFrom._children.put( rn.removedNode.name, rn.removedNode); } } private ParamNode getChild(String[] nestedNames) { ParamNode currentChildNode = this; - for (int i=0; i params) { - RootParamNode root = new RootParamNode( params); + RootParamNode root = new RootParamNode(params); for (Map.Entry e : params.entrySet()) { String key = e.getKey(); @@ -139,8 +139,8 @@ public static RootParamNode convert(Map params) { ParamNode currentParent = root; - for ( String name : key.split(keyPartDelimiterRegexpString)) { - ParamNode paramNode = currentParent.getChild( name ); + for (String name : keyPartDelimiterRegex.split(key)) { + ParamNode paramNode = currentParent.getChild(name); if (paramNode ==null) { // first time we see this node - create it and add it to parent paramNode = new ParamNode(name); @@ -150,8 +150,7 @@ public static RootParamNode convert(Map params) { } // currentParent is now the last node where we should place the values - currentParent.setValue( values, key); - + currentParent.setValue(values, key); } return root; } diff --git a/framework/test/play/data/binding/BinderTest.java b/framework/test/play/data/binding/BinderTest.java index 23b7bda8..51c7f60e 100644 --- a/framework/test/play/data/binding/BinderTest.java +++ b/framework/test/play/data/binding/BinderTest.java @@ -54,16 +54,14 @@ public void verify_unbinding_and_binding_of_simple_Bean() { data1.a = "aAaA"; data1.b = 13; - - Map r = new HashMap<>(); Data1.myStatic = 1; Unbinder.unBind(r, data1, "data1", noAnnotations); - // make sure we only have info about the properties we want.. + // make sure we only have info about the properties we want assertThat(r.keySet()).containsOnly("data1.a", "data1.b"); - Map r2 = fromUnbindMap2BindMap( r); + Map r2 = fromUnbindMap2BindMap(r); Data1.myStatic = 2; RootParamNode root = ParamNode.convert(r2); @@ -72,6 +70,17 @@ public void verify_unbinding_and_binding_of_simple_Bean() { assertThat(Data1.myStatic).isEqualTo(2); } + @Test + public void verify_equality_of_structural_hierarchy() { + + Map abc12 = Map.of("a.b.c", new String[] {"12"}); + RootParamNode root = ParamNode.convert(abc12); + assertThat(root.getChild("a.b.c").getValues()).isEqualTo(new String[] {"12"}); + assertThat(root.getChild("a[b].c").getValues()).isEqualTo(new String[] {"12"}); + assertThat(root.getChild("a[b][c]").getValues()).isEqualTo(new String[] {"12"}); + assertThat(root.getChild("a.b[c]").getValues()).isEqualTo(new String[] {"12"}); + } + @Test public void verify_unbinding_and_binding_of_nestedBeans() { @@ -94,8 +103,6 @@ public void verify_unbinding_and_binding_of_nestedBeans() { data2.data.add(data1_1); data2.data.add(data1_2); - - Map r = new HashMap<>(); Unbinder.unBind(r, data2, "data2", noAnnotations); Map r2 = fromUnbindMap2BindMap(r); @@ -234,8 +241,8 @@ public void test_enum_set_binding() { RootParamNode rootParamNode = ParamNode.convert(params); - Data5 binded = (Data5) Binder.bind(request, session, rootParamNode, "data", Data5.class, Data5.class, noAnnotations); - assertThat(binded.testEnumSet).isEqualTo(data.testEnumSet); + Data5 bound = (Data5) Binder.bind(request, session, rootParamNode, "data", Data5.class, Data5.class, noAnnotations); + assertThat(bound.testEnumSet).isEqualTo(data.testEnumSet); } @Test @@ -245,8 +252,8 @@ public void test_binding_class_with_private_constructor() { RootParamNode rootParamNode = ParamNode.convert(params); - Data6 binded = (Data6) Binder.bind(request, session, rootParamNode, "user", Data6.class, Data6.class, noAnnotations); - assertThat(binded.name).isEqualTo("john"); + Data6 bound = (Data6) Binder.bind(request, session, rootParamNode, "user", Data6.class, Data6.class, noAnnotations); + assertThat(bound.name).isEqualTo("john"); } /** @@ -265,7 +272,7 @@ private Map fromUnbindMap2BindMap(Map r) { } else if (v instanceof String[]) { r2.put(key, (String[])v); } else if (v instanceof Collection) { - Object[] array = ((Collection) v).toArray(); + Object[] array = ((Collection) v).toArray(); r2.put(key, Arrays.copyOf(array, array.length, String[].class)); } else { throw new RuntimeException("error");