-
Notifications
You must be signed in to change notification settings - Fork 191
Builder and extensions cleanup #283
Changes from 2 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