-
Notifications
You must be signed in to change notification settings - Fork 191
Builder and extensions cleanup #283
Changes from all commits
43c3913
7d7cd5f
4030be5
0737ea3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,27 +20,13 @@ public MapWhenMiddleware([NotNull] RequestDelegate next, [NotNull] MapWhenOption | |
|
||
public async Task Invoke([NotNull] HttpContext context) | ||
{ | ||
if (_options.Predicate != null) | ||
if (_options.Predicate(context)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't need a null check anymore? The setter for this property doesn't seem to guard against null. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The NotNull guard is here: It used to check before because there were two different options, sync or async. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's a guard on the method, not on the actual options class. Someone can still go to the options and make it null, no? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With effort. They would have to do There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, true. Can we add a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated. |
||
{ | ||
if (_options.Predicate(context)) | ||
{ | ||
await _options.Branch(context); | ||
} | ||
else | ||
{ | ||
await _next(context); | ||
} | ||
await _options.Branch(context); | ||
} | ||
else | ||
{ | ||
if (await _options.PredicateAsync(context)) | ||
{ | ||
await _options.Branch(context); | ||
} | ||
else | ||
{ | ||
await _next(context); | ||
} | ||
await _next(context); | ||
} | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We totally need to change this namespace