Skip to content
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

Improve performance, add test, small cleanups #364

Merged
merged 1 commit into from
Mar 29, 2024
Merged

Improve performance, add test, small cleanups #364

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
37 changes: 18 additions & 19 deletions framework/src/play/data/binding/ParamNode.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,21 @@
import play.utils.Utils;

import java.util.*;
import java.util.regex.Pattern;

public class ParamNode {
private final String name;
private final Map<String, ParamNode> _children = new HashMap<>(8);
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;
Expand All @@ -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);
}

Expand All @@ -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);
}
Expand All @@ -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.
*
Expand All @@ -82,24 +82,24 @@ public RemovedNode(ParamNode removedFrom, ParamNode removedNode) {
*/
public boolean removeChild(String name, List<RemovedNode> 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;
}
}

public static void restoreRemovedChildren( List<RemovedNode> 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<nestedNames.length; i++) {
currentChildNode = currentChildNode._children.get(nestedNames[i]);
for (String nestedName : nestedNames) {
currentChildNode = currentChildNode._children.get(nestedName);
if (currentChildNode == null) {
return null;
}
Expand Down Expand Up @@ -128,7 +128,7 @@ public String getOriginalKey() {
}

public static RootParamNode convert(Map<String, String[]> params) {
RootParamNode root = new RootParamNode( params);
RootParamNode root = new RootParamNode(params);

for (Map.Entry<String, String[]> e : params.entrySet()) {
String key = e.getKey();
Expand All @@ -139,8 +139,8 @@ public static RootParamNode convert(Map<String, String[]> 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);
Expand All @@ -150,8 +150,7 @@ public static RootParamNode convert(Map<String, String[]> params) {
}

// currentParent is now the last node where we should place the values
currentParent.setValue( values, key);

currentParent.setValue(values, key);
}
return root;
}
Expand Down
29 changes: 18 additions & 11 deletions framework/test/play/data/binding/BinderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,16 +54,14 @@ public void verify_unbinding_and_binding_of_simple_Bean() {
data1.a = "aAaA";
data1.b = 13;



Map<String, Object> 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<String, String[]> r2 = fromUnbindMap2BindMap( r);
Map<String, String[]> r2 = fromUnbindMap2BindMap(r);

Data1.myStatic = 2;
RootParamNode root = ParamNode.convert(r2);
Expand All @@ -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<String, String[]> 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() {
Expand All @@ -94,8 +103,6 @@ public void verify_unbinding_and_binding_of_nestedBeans() {
data2.data.add(data1_1);
data2.data.add(data1_2);



Map<String, Object> r = new HashMap<>();
Unbinder.unBind(r, data2, "data2", noAnnotations);
Map<String, String[]> r2 = fromUnbindMap2BindMap(r);
Expand Down Expand Up @@ -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
Expand All @@ -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");
}

/**
Expand All @@ -265,7 +272,7 @@ private Map<String, String[]> fromUnbindMap2BindMap(Map<String, Object> 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");
Expand Down