Skip to content

Commit dc38ae8

Browse files
authored
chore: More spanner perf (#131)
* simplify lookup func * _ * _ * remove descriptors > bad feature * _ * improve param matching > more perf * use next route part to match early * add look ahead * defer params map to later * tiny fix * _
1 parent 18210de commit dc38ae8

File tree

11 files changed

+335
-307
lines changed

11 files changed

+335
-307
lines changed

packages/pharaoh/lib/src/_next/core.dart

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -198,10 +198,6 @@ abstract class ApplicationFactory {
198198
final exception = error.exception;
199199
if (exception is RequestValidationError) {
200200
return response.json(exception, statusCode: HttpStatus.badRequest);
201-
} else if (error is SpannerRouteValidatorError) {
202-
return response.json({
203-
'errors': [exception]
204-
}, statusCode: HttpStatus.badRequest);
205201
}
206202
return response.internalServerError(exception.toString());
207203
}

packages/pharaoh/lib/src/core_impl.dart

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -82,17 +82,14 @@ class $PharaohImpl extends RouterContract
8282
}
8383

8484
if (_onErrorCb == null) {
85-
final status = requestError.exception is SpannerRouteValidatorError
86-
? HttpStatus.unprocessableEntity
87-
: HttpStatus.internalServerError;
8885
return forward(
8986
httpReq,
9087
response.json(
9188
{
9289
'error': requestError.exception.toString(),
9390
'trace': requestError.trace.toString()
9491
},
95-
statusCode: status,
92+
statusCode: HttpStatus.internalServerError,
9693
),
9794
);
9895
}

packages/pharaoh/test/acceptance/request_handling_test.dart

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -9,51 +9,46 @@ void main() {
99
..get('/home/chima', (req, res) => res.ok('Okay 🚀'))
1010
..delete('/home/chima', (req, res) => res.ok('Item deleted'))
1111
..post('/home/strange', (req, res) => res.ok('Post something 🚀'))
12-
..get('/chima/<userId|number>', (req, res) => res.ok('Foo Bar'));
12+
..get('/chima/<userId>', (req, res) => res.json(req.params));
1313

14-
await (await request(app))
14+
final tester = await request(app);
15+
16+
await tester
1517
.get('/home/chima')
1618
.expectStatus(200)
1719
.expectBody('Okay 🚀')
1820
.test();
1921

20-
await (await request(app))
22+
await tester
2123
.post('/home/strange', {})
2224
.expectStatus(200)
2325
.expectBody('Post something 🚀')
2426
.test();
2527

26-
await (await request(app))
28+
await tester
2729
.get('/users/204')
2830
.expectStatus(200)
2931
.expectBody({'userId': '204'}).test();
3032

31-
await (await request(app))
33+
await tester
3234
.post('/users/204398938948374797', {})
3335
.expectStatus(200)
3436
.expectBody({'userId': '204398938948374797'})
3537
.test();
3638

37-
await (await request(app))
38-
.get('/something-new-is-here')
39-
.expectStatus(404)
40-
.expectBody(
41-
{"error": "Route not found: /something-new-is-here"}).test();
39+
await tester.get('/something-new-is-here').expectStatus(404).expectBody(
40+
{"error": "Route not found: /something-new-is-here"}).test();
4241

43-
await (await request(app))
42+
await tester
4443
.delete('/home/chima')
4544
.expectStatus(200)
4645
.expectBody('Item deleted')
4746
.test();
4847

49-
await (await request(app))
48+
await tester
5049
.get('/chima/asbc')
51-
.expectStatus(422)
52-
.expectJsonBody(containsPair(
53-
'error',
54-
'Invalid argument: Invalid parameter value: \"asbc\"',
55-
))
56-
.test();
50+
.expectStatus(200)
51+
.expectJsonBody({'userId': 'asbc'}).test();
5752
});
5853

5954
group('execute middleware and request', () {

packages/spanner/lib/spanner.dart

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
11
library;
22

33
export 'src/tree/tree.dart' show RouterConfig, RouteResult, HTTPMethod, Spanner;
4-
export 'src/parametric/descriptor.dart' show SpannerRouteValidatorError;

packages/spanner/lib/src/parametric/definition.dart

Lines changed: 80 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -1,41 +1,18 @@
11
import '../tree/node.dart';
22
import '../tree/tree.dart';
3-
import 'descriptor.dart';
43
import 'utils.dart';
54

6-
final _knownDescriptors = {'number': numDescriptor};
5+
typedef ParamAndValue = ({String name, String? value});
76

8-
SingleParameterDefn _singleParamDefn(RegExpMatch m) {
9-
final group = m.group(2)!;
10-
var param = group;
7+
SingleParameterDefn _singleParamDefn(RegExpMatch m, [String? nextPart]) =>
8+
SingleParameterDefn._(
9+
m.group(2)!,
10+
prefix: m.group(1)?.nullIfEmpty,
11+
suffix: m.group(3)?.nullIfEmpty,
12+
nextPart: nextPart,
13+
);
1114

12-
Iterable<ParameterDescriptor>? descriptors;
13-
14-
if (group.contains('|')) {
15-
final parts = m.group(2)!.split('|');
16-
param = parts.first;
17-
18-
if (parts.length > 1) {
19-
descriptors = parts.sublist(1).map((e) {
20-
final value = e.isRegex ? regexDescriptor : _knownDescriptors[e];
21-
if (value == null) {
22-
throw ArgumentError.value(
23-
e, null, 'Parameter definition has invalid descriptor');
24-
}
25-
return value;
26-
});
27-
}
28-
}
29-
30-
return SingleParameterDefn._(
31-
param,
32-
prefix: m.group(1)?.nullIfEmpty,
33-
suffix: m.group(3)?.nullIfEmpty,
34-
descriptors: descriptors ?? const [],
35-
);
36-
}
37-
38-
ParameterDefinition buildParamDefinition(String part) {
15+
ParameterDefinition buildParamDefinition(String part, [String? nextPart]) {
3916
if (closeDoorParametricRegex.hasMatch(part)) {
4017
throw ArgumentError.value(
4118
part, null, 'Parameter definition is invalid. Close door neighbors');
@@ -47,27 +24,34 @@ ParameterDefinition buildParamDefinition(String part) {
4724
}
4825

4926
if (matches.length == 1) {
50-
return _singleParamDefn(matches.first);
27+
return _singleParamDefn(matches.first, nextPart);
5128
}
5229

53-
final defns = matches.map(_singleParamDefn);
54-
final partsMap = {for (final defn in defns) defn.name: defn};
55-
56-
return CompositeParameterDefinition._(partsMap, defns.last.name);
30+
return CompositeParameterDefinition._(
31+
matches.map(_singleParamDefn),
32+
nextPart,
33+
);
5734
}
5835

5936
abstract class ParameterDefinition implements HandlerStore {
6037
String get name;
6138

6239
String get templateStr;
6340

64-
RegExp get template;
65-
6641
String get key;
6742

6843
bool get terminal;
6944

70-
Map<String, dynamic>? resolveParams(String pattern);
45+
/// Next route part included so we can match early if its a static next part
46+
String? get nextPart;
47+
48+
bool matches(String route, {bool caseSensitive = false});
49+
50+
void resolveParams(
51+
String pattern,
52+
List<ParamAndValue> collector, {
53+
bool caseSentive = false,
54+
});
7155
}
7256

7357
class SingleParameterDefn extends ParameterDefinition with HandlerStoreMixin {
@@ -77,13 +61,11 @@ class SingleParameterDefn extends ParameterDefinition with HandlerStoreMixin {
7761
final String? prefix;
7862
final String? suffix;
7963

80-
final Iterable<ParameterDescriptor> descriptors;
81-
8264
@override
83-
final String templateStr;
65+
final String? nextPart;
8466

8567
@override
86-
late final RegExp template;
68+
final String templateStr;
8769

8870
@override
8971
String get key => 'prefix=$prefix&suffix=$suffix&terminal=$terminal';
@@ -93,32 +75,36 @@ class SingleParameterDefn extends ParameterDefinition with HandlerStoreMixin {
9375
@override
9476
bool get terminal => _terminal;
9577

78+
@override
79+
bool matches(String route, {bool caseSensitive = false}) {
80+
return matchPattern(
81+
route,
82+
prefix ?? '',
83+
suffix ?? '',
84+
caseSensitive,
85+
) !=
86+
null;
87+
}
88+
9689
SingleParameterDefn._(
9790
this.name, {
98-
this.descriptors = const [],
9991
this.prefix,
10092
this.suffix,
101-
}) : templateStr = buildTemplateString(
102-
name: name,
103-
prefix: prefix,
104-
suffix: suffix,
105-
),
106-
_terminal = false {
107-
template = buildRegexFromTemplate(templateStr);
108-
}
109-
110-
bool matches(String pattern) => template.hasMatch(pattern);
111-
112-
@override
113-
Map<String, dynamic>? resolveParams(final String pattern) {
114-
final params = resolveParamsFromPath(template, pattern);
115-
if (params == null) return null;
116-
117-
return params
118-
..[name] = descriptors.fold(
119-
params[name],
120-
(value, descriptor) => descriptor(value),
121-
);
93+
this.nextPart,
94+
}) : templateStr =
95+
buildTemplateString(name: name, prefix: prefix, suffix: suffix),
96+
_terminal = false;
97+
98+
@override
99+
void resolveParams(
100+
final String pattern,
101+
List<ParamAndValue> collector, {
102+
bool caseSentive = false,
103+
}) {
104+
collector.add((
105+
name: name,
106+
value: matchPattern(pattern, prefix ?? "", suffix ?? "", caseSentive)
107+
));
122108
}
123109

124110
@override
@@ -130,44 +116,45 @@ class SingleParameterDefn extends ParameterDefinition with HandlerStoreMixin {
130116

131117
class CompositeParameterDefinition extends ParameterDefinition
132118
implements HandlerStore {
133-
final Map<String, SingleParameterDefn> parts;
134-
final String _lastPartKey;
119+
final Iterable<SingleParameterDefn> parts;
120+
final SingleParameterDefn _maybeTerminalPart;
135121

136-
SingleParameterDefn get _maybeTerminalPart => parts[_lastPartKey]!;
122+
@override
123+
final String? nextPart;
137124

138-
CompositeParameterDefinition._(this.parts, this._lastPartKey);
125+
CompositeParameterDefinition._(this.parts, [this.nextPart])
126+
: _maybeTerminalPart = parts.last;
139127

140128
@override
141-
String get templateStr => parts.values.map((e) => e.templateStr).join();
129+
String get templateStr => parts.map((e) => e.templateStr).join();
142130

143131
@override
144-
String get name => parts.values.map((e) => e.name).join('|');
132+
String get name => parts.map((e) => e.name).join('|');
145133

146134
@override
147-
String get key => parts.values.map((e) => e.key).join('|');
135+
String get key => parts.map((e) => e.key).join('|');
148136

149-
@override
150-
RegExp get template => buildRegexFromTemplate(templateStr);
137+
RegExp get _template => buildRegexFromTemplate(templateStr);
151138

152139
@override
153140
bool get terminal => _maybeTerminalPart.terminal;
154141

155142
@override
156-
Map<String, dynamic>? resolveParams(String pattern) {
157-
final result = resolveParamsFromPath(template, pattern);
158-
if (result == null) return null;
143+
bool matches(String route, {bool caseSensitive = false}) =>
144+
_template.hasMatch(route);
159145

160-
for (final param in result.keys) {
161-
final defn = parts[param]!;
162-
final value = result[param];
146+
@override
147+
void resolveParams(
148+
String pattern,
149+
List<ParamAndValue> collector, {
150+
bool caseSentive = false,
151+
}) {
152+
final match = _template.firstMatch(pattern);
153+
if (match == null) return;
163154

164-
result[param] = defn.descriptors.fold<dynamic>(
165-
value,
166-
(value, fn) => fn(value),
167-
);
155+
for (final key in match.groupNames) {
156+
collector.add((name: key, value: match.namedGroup(key)));
168157
}
169-
170-
return result;
171158
}
172159

173160
@override
@@ -176,12 +163,14 @@ class CompositeParameterDefinition extends ParameterDefinition
176163
}
177164

178165
@override
179-
void addRoute<T>(HTTPMethod method, IndexedValue<T> handler) =>
180-
_maybeTerminalPart.addRoute(method, handler);
166+
void addRoute<T>(HTTPMethod method, IndexedValue<T> handler) {
167+
_maybeTerminalPart.addRoute(method, handler);
168+
}
181169

182170
@override
183-
IndexedValue? getHandler(HTTPMethod method) =>
184-
_maybeTerminalPart.getHandler(method);
171+
IndexedValue? getHandler(HTTPMethod method) {
172+
return _maybeTerminalPart.getHandler(method);
173+
}
185174

186175
@override
187176
bool hasMethod(HTTPMethod method) => _maybeTerminalPart.hasMethod(method);

packages/spanner/lib/src/parametric/descriptor.dart

Lines changed: 0 additions & 25 deletions
This file was deleted.

0 commit comments

Comments
 (0)