Skip to content
This repository has been archived by the owner on Nov 22, 2018. It is now read-only.

Conditional requests #22

Merged
merged 1 commit into from
Aug 31, 2016
Merged

Conditional requests #22

merged 1 commit into from
Aug 31, 2016

Conversation

JunTaoLuo
Copy link
Contributor

Add support to send 304 responses for requests conditional on IfModifiedSince and IfNoneMatch.

cc @Tratcher @davidfowl

@JunTaoLuo JunTaoLuo force-pushed the johluo/conditional-requests branch from 1382273 to 2c1b708 Compare August 29, 2016 23:42
@JunTaoLuo JunTaoLuo mentioned this pull request Aug 29, 2016
34 tasks
if (ConditionalRequestSatisfied(cachedResponseHeaders))
{
_httpContext.Response.StatusCode = StatusCodes.Status304NotModified;
return true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

inconsistent use of responseServed vs return true.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had the responseServed variable previously because we couldn't short-circuit after failing to serve a response due to the OnlyIfCached header. The conditional requests are different, if we served that, we can return right away. But I guess for consistency, I can just keep using responseServed and return it at the end. This means one extra check on the boolean even after satisfying the conditional request but I guess that's fine.

@JunTaoLuo JunTaoLuo force-pushed the johluo/conditional-requests branch from eb8426a to 916d12e Compare August 30, 2016 22:23
@JunTaoLuo
Copy link
Contributor Author

🆙📅

{
foreach (var tag in ifNoneMatchHeader)
{
// TODO: use strong comparison
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This TODO is complete

@Tratcher
Copy link
Member

:shipit:
What about all the other conditionals?

@JunTaoLuo JunTaoLuo force-pushed the johluo/conditional-requests branch from 916d12e to 52f219b Compare August 31, 2016 21:45
@JunTaoLuo JunTaoLuo merged commit 52f219b into dev Aug 31, 2016
@JunTaoLuo JunTaoLuo deleted the johluo/conditional-requests branch August 31, 2016 21:45
@JunTaoLuo
Copy link
Contributor Author

Only IfNoneMatch and IfModifiedSince applies to us for now.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants