Skip to content

Commit a5ef6af

Browse files
jknackjknack
committed
bean/form parser enhancements fix jooby-project#479 
1 parent abec5e8 commit a5ef6af

File tree

10 files changed

+473
-127
lines changed

10 files changed

+473
-127
lines changed

‎coverage-report/src/test/java/org/jooby/BeanParserFeature.java‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,11 @@ public String getName() {
5050

5151
}
5252

53-
public static class InvalidBean {
54-
public InvalidBean(final String arg) {
53+
public static class DefConstBean {
54+
public DefConstBean(final String arg) {
5555
}
5656

57-
public InvalidBean() {
57+
public DefConstBean() {
5858
}
5959
}
6060

@@ -114,8 +114,8 @@ public String postbeannoarg(final org.jooby.Request req, final BeanNoArg bean) t
114114
}
115115

116116
@org.jooby.mvc.GET
117-
@Path("/invalidbean")
118-
public String invalidbean(final InvalidBean bean) throws Exception {
117+
@Path("/defaultConstructor")
118+
public String invalidbean(final DefConstBean bean) throws Exception {
119119
return "OK";
120120
}
121121

@@ -145,8 +145,8 @@ public String invalidbean(final InvalidBean bean) throws Exception {
145145
return "OK";
146146
});
147147

148-
get("/invalidbean", req -> {
149-
req.params().to(InvalidBean.class);
148+
get("/defaultConstructor", req -> {
149+
req.params().to(DefConstBean.class);
150150
return "OK";
151151
});
152152

@@ -211,14 +211,14 @@ public void getbeannoarg() throws Exception {
211211
}
212212

213213
@Test
214-
public void invalidbean() throws Exception {
214+
public void defaultConstructor() throws Exception {
215215
request()
216-
.get("/invalidbean?name=edgar&age=17")
217-
.expect(400);
216+
.get("/defaultConstructor?name=edgar&age=17")
217+
.expect(200);
218218

219219
request()
220-
.get("/r/invalidbean?name=edgar&age=17")
221-
.expect(400);
220+
.get("/r/defaultConstructor?name=edgar&age=17")
221+
.expect(200);
222222

223223
}
224224

‎coverage-report/src/test/java/org/jooby/ParserDefOrderFeature.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,6 @@ public class ParserDefOrderFeature extends ServerFeature {
2323
public void order() throws Exception {
2424
request()
2525
.get("/parser/order")
26-
.expect("[Basic, Collection, Optional, Enum, Upload, byte[], Date, LocalDate, Locale, bean, valueOf(String), fromString(String), forName(String), init(String)]");
26+
.expect("[Basic, Collection, Optional, Enum, Upload, byte[], Date, LocalDate, Locale, valueOf(String), fromString(String), forName(String), init(String), bean]");
2727
}
2828
}

‎coverage-report/src/test/java/org/jooby/ParserOrder2Feature.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public String toString() {
3939
@Override
4040
public Object parse(final TypeLiteral<?> type, final Context ctx) throws Throwable {
4141
assertEquals(
42-
"[Basic, Collection, Optional, Enum, Upload, byte[], p2, p1, p3, Date, LocalDate, Locale, bean, valueOf(String), fromString(String), forName(String), init(String)]",
42+
"[Basic, Collection, Optional, Enum, Upload, byte[], p2, p1, p3, Date, LocalDate, Locale, valueOf(String), fromString(String), forName(String), init(String), bean]",
4343
ctx.toString());
4444
return ctx.next();
4545
}
@@ -74,6 +74,6 @@ public void order() throws Exception {
7474
request()
7575
.get("/parser/order")
7676
.expect(
77-
"[Basic, Collection, Optional, Enum, Upload, byte[], p2, p1, p3, Date, LocalDate, Locale, bean, valueOf(String), fromString(String), forName(String), init(String)]");
77+
"[Basic, Collection, Optional, Enum, Upload, byte[], p2, p1, p3, Date, LocalDate, Locale, valueOf(String), fromString(String), forName(String), init(String), bean]");
7878
}
7979
}

‎coverage-report/src/test/java/org/jooby/ParserOrderFeature.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ public class ParserOrderFeature extends ServerFeature {
2323
@Override
2424
public Object parse(final TypeLiteral<?> type, final Context ctx) throws Throwable {
2525
assertEquals(
26-
"[Basic, Collection, Optional, Enum, Upload, byte[], p1, p2, p3, Date, LocalDate, Locale, bean, valueOf(String), fromString(String), forName(String), init(String)]",
26+
"[Basic, Collection, Optional, Enum, Upload, byte[], p1, p2, p3, Date, LocalDate, Locale, valueOf(String), fromString(String), forName(String), init(String), bean]",
2727
ctx.toString());
2828
return ctx.next();
2929
}
@@ -75,6 +75,6 @@ public void order() throws Exception {
7575
request()
7676
.get("/parser/order")
7777
.expect(
78-
"[Basic, Collection, Optional, Enum, Upload, byte[], p1, p2, p3, Date, LocalDate, Locale, bean, valueOf(String), fromString(String), forName(String), init(String)]");
78+
"[Basic, Collection, Optional, Enum, Upload, byte[], p1, p2, p3, Date, LocalDate, Locale, valueOf(String), fromString(String), forName(String), init(String), bean]");
7979
}
8080
}

‎jooby/src/main/java/org/jooby/Jooby.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3409,6 +3409,7 @@ public Jooby err(final Err.Handler err) {
34093409
* @param handler A connect callback.
34103410
* @return A new WebSocket definition.
34113411
*/
3412+
@Override
34123413
public WebSocket.Definition ws(final String path, final WebSocket.FullHandler handler) {
34133414
WebSocket.Definition ws = new WebSocket.Definition(path, handler);
34143415
checkArgument(bag.add(ws), "Path is in use: '%s'", path);
@@ -4163,11 +4164,11 @@ private Injector bootstrap(final Config args,
41634164
parsers.addBinding().toInstance(new DateParser(dateFormat));
41644165
parsers.addBinding().toInstance(new LocalDateParser(dateTimeFormatter));
41654166
parsers.addBinding().toInstance(new LocaleParser());
4166-
parsers.addBinding().toInstance(beanParser.orElseGet(() -> new BeanParser(false)));
41674167
parsers.addBinding().toInstance(new StaticMethodParser("valueOf"));
41684168
parsers.addBinding().toInstance(new StaticMethodParser("fromString"));
41694169
parsers.addBinding().toInstance(new StaticMethodParser("forName"));
41704170
parsers.addBinding().toInstance(new StringConstructorParser());
4171+
parsers.addBinding().toInstance(beanParser.orElseGet(() -> new BeanParser(false)));
41714172

41724173
binder.bind(ParserExecutor.class).in(Singleton.class);
41734174

‎jooby/src/main/java/org/jooby/internal/parser/BeanParser.java‎

Lines changed: 12 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -18,17 +18,9 @@
1818
*/
1919
package org.jooby.internal.parser;
2020

21-
import java.lang.reflect.Constructor;
22-
import java.lang.reflect.Field;
23-
import java.lang.reflect.Modifier;
24-
import java.util.Arrays;
25-
import java.util.List;
2621
import java.util.Map;
27-
import java.util.Map.Entry;
22+
import java.util.concurrent.ConcurrentHashMap;
2823
import java.util.function.Function;
29-
import java.util.stream.Collectors;
30-
31-
import javax.inject.Inject;
3224

3325
import org.jooby.Err;
3426
import org.jooby.Mutant;
@@ -37,13 +29,7 @@
3729
import org.jooby.Response;
3830
import org.jooby.internal.ParameterNameProvider;
3931
import org.jooby.internal.mvc.RequestParam;
40-
import org.jooby.internal.mvc.RequestParamNameProviderImpl;
41-
import org.jooby.internal.mvc.RequestParamProvider;
42-
import org.jooby.internal.mvc.RequestParamProviderImpl;
43-
import org.slf4j.LoggerFactory;
4432

45-
import com.google.common.base.CharMatcher;
46-
import com.google.common.base.Splitter;
4733
import com.google.common.primitives.Primitives;
4834
import com.google.common.reflect.Reflection;
4935
import com.google.inject.TypeLiteral;
@@ -60,8 +46,12 @@ public class BeanParser implements Parser {
6046

6147
private Function<? super Throwable, Try<? extends Object>> recoverMissing;
6248

49+
@SuppressWarnings("rawtypes")
50+
private final Map<Class, BeanPlan> beans;
51+
6352
public BeanParser(final boolean allowNulls) {
6453
this.recoverMissing = allowNulls ? MISSING : RETHROW;
54+
this.beans = new ConcurrentHashMap<>();
6555
}
6656

6757
@Override
@@ -79,7 +69,7 @@ public Object parse(final TypeLiteral<?> type, final Context ctx) throws Throwab
7969
bean = newBean(ctx.require(Request.class), ctx.require(Response.class), map, beanType);
8070
}
8171

82-
return bean == null ? ctx.next() : bean;
72+
return bean;
8373
});
8474
}
8575

@@ -90,75 +80,13 @@ public String toString() {
9080

9181
private Object newBean(final Request req, final Response rsp,
9282
final Map<String, Mutant> params, final Class<?> beanType) throws Throwable {
93-
ParameterNameProvider classInfo = req.require(ParameterNameProvider.class);
94-
List<Constructor<?>> constructors = Arrays.asList(beanType.getDeclaredConstructors()).stream()
95-
.filter(c -> c.isAnnotationPresent(Inject.class))
96-
.collect(Collectors.toList());
97-
if (constructors.size() == 0) {
98-
// No inject annotation, use a declared constructor
99-
constructors.addAll(Arrays.asList(beanType.getDeclaredConstructors()));
100-
}
101-
if (constructors.size() > 1) {
102-
return null;
103-
}
104-
Constructor<?> constructor = constructors.get(0);
105-
RequestParamProvider provider = new RequestParamProviderImpl(
106-
new RequestParamNameProviderImpl(classInfo));
107-
List<RequestParam> parameters = provider.parameters(constructor);
108-
Object[] args = new Object[parameters.size()];
109-
for (int i = 0; i < args.length; i++) {
110-
args[i] = value(parameters.get(i), req, rsp);
111-
}
112-
// inject args
113-
final Object bean = constructor.newInstance(args);
114-
115-
// inject fields
116-
for (Entry<String, Mutant> param : params.entrySet()) {
117-
String pname = param.getKey();
118-
try {
119-
List<String> path = name(pname);
120-
Object root = seek(bean, path);
121-
String fname = path.get(path.size() - 1);
122-
123-
Field field = root.getClass().getDeclaredField(fname);
124-
int mods = field.getModifiers();
125-
if (!Modifier.isFinal(mods) && !Modifier.isStatic(mods) && !Modifier.isTransient(mods)) {
126-
// get
127-
Object value = value(new RequestParam(field, pname, field.getGenericType()), req, rsp);
128-
// set
129-
field.setAccessible(true);
130-
field.set(root, value);
131-
}
132-
} catch (NoSuchFieldException ex) {
133-
LoggerFactory.getLogger(Request.class).debug("No matching field for: {}", pname);
134-
}
83+
BeanPlan plan = beans.get(beanType);
84+
if (plan == null) {
85+
ParameterNameProvider classInfo = req.require(ParameterNameProvider.class);
86+
plan = new BeanPlan(classInfo, beanType);
87+
beans.put(beanType, plan);
13588
}
136-
return bean;
137-
}
138-
139-
/**
140-
* Given a path like: <code>profile[address][country][name]</code> this method will traverse the
141-
* path and seek the object in [country].
142-
*
143-
* @param bean Root bean.
144-
* @param path Path to traverse.
145-
* @return The last object in the path.
146-
* @throws Exception If something goes wrong.
147-
*/
148-
private Object seek(final Object bean, final List<String> path) throws Exception {
149-
Object it = bean;
150-
for (int i = 0; i < path.size() - 1; i++) {
151-
Field field = it.getClass().getDeclaredField(path.get(i));
152-
field.setAccessible(true);
153-
154-
Object next = field.get(it);
155-
if (next == null) {
156-
next = field.getType().newInstance();
157-
field.set(it, next);
158-
}
159-
it = next;
160-
}
161-
return it;
89+
return plan.newBean(p -> value(p, req, rsp), params);
16290
}
16391

16492
private Object newBeanInterface(final Request req, final Response rsp, final Class<?> beanType) {
@@ -179,12 +107,4 @@ private Object value(final RequestParam param, final Request req, final Response
179107
.getOrElseThrow(Function.identity());
180108
}
181109

182-
private static List<String> name(final String name) {
183-
return Splitter.on(new CharMatcher() {
184-
@Override
185-
public boolean matches(final char c) {
186-
return c == '[' || c == ']';
187-
}
188-
}).trimResults().omitEmptyStrings().splitToList(name);
189-
}
190110
}

0 commit comments

Comments
 (0)