Skip to content

Commit

Permalink
caching: Refactored registration of inline headers in the CacheFilter…
Browse files Browse the repository at this point in the history
… tree to a separate header file (#12576)

Refactored registration of inline headers in the CacheFilter tree to a separate header file.

Signed-off-by: Yosry Ahmed <yosryahmed@google.com>
  • Loading branch information
yosrym93 authored Aug 12, 2020
1 parent ea7665a commit a7e7bd1
Show file tree
Hide file tree
Showing 6 changed files with 72 additions and 25 deletions.
12 changes: 11 additions & 1 deletion source/extensions/filters/http/cache/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ envoy_cc_library(
":cache_headers_utils_lib",
":cacheability_utils_lib",
":http_cache_lib",
":inline_headers_handles",
"//source/common/common:enum_to_int",
"//source/common/common:logger_lib",
"//source/common/common:macros",
"//source/common/http:header_map_lib",
"//source/common/http:headers_lib",
"//source/common/http:utility_lib",
"//source/extensions/filters/http/common:pass_through_filter_lib",
Expand All @@ -37,6 +37,7 @@ envoy_cc_library(
hdrs = ["cacheability_utils.h"],
deps = [
":cache_headers_utils_lib",
":inline_headers_handles",
"//source/common/common:utility_lib",
"//source/common/http:headers_lib",
],
Expand All @@ -53,6 +54,7 @@ envoy_cc_library(
hdrs = ["http_cache.h"],
deps = [
":cache_headers_utils_lib",
":inline_headers_handles",
":key_cc_proto",
"//include/envoy/buffer:buffer_interface",
"//include/envoy/common:time_interface",
Expand All @@ -78,6 +80,14 @@ envoy_cc_library(
],
)

envoy_cc_library(
name = "inline_headers_handles",
hdrs = ["inline_headers_handles.h"],
deps = [
"//source/common/http:headers_lib",
],
)

envoy_cc_extension(
name = "config",
srcs = ["config.cc"],
Expand Down
17 changes: 8 additions & 9 deletions source/extensions/filters/http/cache/cache_filter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "common/http/utility.h"

#include "extensions/filters/http/cache/cacheability_utils.h"
#include "extensions/filters/http/cache/inline_headers_handles.h"

#include "absl/strings/string_view.h"

Expand Down Expand Up @@ -274,9 +275,8 @@ bool CacheFilter::shouldUpdateCachedEntry(const Http::ResponseHeaderMap& respons
// and assuming a single cached response per key:
// If the 304 response contains a strong validator (etag) that does not match the cached response,
// the cached response should not be updated.
const Http::HeaderEntry* response_etag = response_headers.get(Http::CustomHeaders::get().Etag);
const Http::HeaderEntry* cached_etag =
lookup_result_->headers_->get(Http::CustomHeaders::get().Etag);
const Http::HeaderEntry* response_etag = response_headers.getInline(etag_handle.handle());
const Http::HeaderEntry* cached_etag = lookup_result_->headers_->getInline(etag_handle.handle());
return !response_etag || (cached_etag && cached_etag->value().getStringView() ==
response_etag->value().getStringView());
}
Expand All @@ -288,25 +288,24 @@ void CacheFilter::injectValidationHeaders(Http::RequestHeaderMap& request_header
"injectValidationHeaders precondition unsatisfied: the "
"CacheFilter is not validating a cache lookup result");

const Http::HeaderEntry* etag_header =
lookup_result_->headers_->get(Http::CustomHeaders::get().Etag);
const Http::HeaderEntry* etag_header = lookup_result_->headers_->getInline(etag_handle.handle());
const Http::HeaderEntry* last_modified_header =
lookup_result_->headers_->get(Http::CustomHeaders::get().LastModified);
lookup_result_->headers_->getInline(last_modified_handle.handle());

if (etag_header) {
absl::string_view etag = etag_header->value().getStringView();
request_headers.setReferenceKey(Http::CustomHeaders::get().IfNoneMatch, etag);
request_headers.setInline(if_none_match_handle.handle(), etag);
}
if (CacheHeadersUtils::httpTime(last_modified_header) != SystemTime()) {
// Valid Last-Modified header exists.
absl::string_view last_modified = last_modified_header->value().getStringView();
request_headers.setReferenceKey(Http::CustomHeaders::get().IfModifiedSince, last_modified);
request_headers.setInline(if_modified_since_handle.handle(), last_modified);
} else {
// Either Last-Modified is missing or invalid, fallback to Date.
// A correct behaviour according to:
// https://httpwg.org/specs/rfc7232.html#header.if-modified-since
absl::string_view date = lookup_result_->headers_->getDateValue();
request_headers.setReferenceKey(Http::CustomHeaders::get().IfModifiedSince, date);
request_headers.setInline(if_modified_since_handle.handle(), date);
}
}

Expand Down
1 change: 0 additions & 1 deletion source/extensions/filters/http/cache/cache_filter.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
#include <vector>

#include "envoy/extensions/filters/http/cache/v3alpha/cache.pb.h"
#include "envoy/http/header_map.h"

#include "common/common/logger.h"

Expand Down
9 changes: 3 additions & 6 deletions source/extensions/filters/http/cache/cacheability_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
#include "common/common/macros.h"
#include "common/common/utility.h"

#include "extensions/filters/http/cache/inline_headers_handles.h"

namespace Envoy {
namespace Extensions {
namespace HttpFilters {
Expand All @@ -30,11 +32,6 @@ const std::vector<const Http::LowerCaseString*>& conditionalHeaders() {
}
} // namespace

Http::RegisterCustomInlineHeader<Http::CustomInlineHeaderRegistry::Type::RequestHeaders>
authorization_handle(Http::CustomHeaders::get().Authorization);
Http::RegisterCustomInlineHeader<Http::CustomInlineHeaderRegistry::Type::ResponseHeaders>
cache_control_handle(Http::CustomHeaders::get().CacheControl);

bool CacheabilityUtils::isCacheableRequest(const Http::RequestHeaderMap& headers) {
const absl::string_view method = headers.getMethodValue();
const absl::string_view forwarded_proto = headers.getForwardedProtoValue();
Expand All @@ -60,7 +57,7 @@ bool CacheabilityUtils::isCacheableRequest(const Http::RequestHeaderMap& headers
}

bool CacheabilityUtils::isCacheableResponse(const Http::ResponseHeaderMap& headers) {
absl::string_view cache_control = headers.getInlineValue(cache_control_handle.handle());
absl::string_view cache_control = headers.getInlineValue(response_cache_control_handle.handle());
ResponseCacheControl response_cache_control(cache_control);

// Only cache responses with explicit validation data, either:
Expand Down
11 changes: 3 additions & 8 deletions source/extensions/filters/http/cache/http_cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
#include "common/http/headers.h"
#include "common/protobuf/utility.h"

#include "extensions/filters/http/cache/inline_headers_handles.h"

#include "absl/strings/str_split.h"
#include "absl/strings/string_view.h"
#include "absl/time/time.h"
Expand All @@ -20,13 +22,6 @@ namespace Extensions {
namespace HttpFilters {
namespace Cache {

Http::RegisterCustomInlineHeader<Http::CustomInlineHeaderRegistry::Type::RequestHeaders>
request_cache_control_handle(Http::CustomHeaders::get().CacheControl);
Http::RegisterCustomInlineHeader<Http::CustomInlineHeaderRegistry::Type::ResponseHeaders>
response_cache_control_handle(Http::CustomHeaders::get().CacheControl);
Http::RegisterCustomInlineHeader<Http::CustomInlineHeaderRegistry::Type::RequestHeaders>
pragma_handler(Http::CustomHeaders::get().Pragma);

std::ostream& operator<<(std::ostream& os, CacheEntryStatus status) {
switch (status) {
case CacheEntryStatus::Ok:
Expand Down Expand Up @@ -90,7 +85,7 @@ size_t localHashKey(const Key& key) { return stableHashKey(key); }
void LookupRequest::initializeRequestCacheControl(const Http::RequestHeaderMap& request_headers) {
const absl::string_view cache_control =
request_headers.getInlineValue(request_cache_control_handle.handle());
const absl::string_view pragma = request_headers.getInlineValue(pragma_handler.handle());
const absl::string_view pragma = request_headers.getInlineValue(pragma_handle.handle());

if (!cache_control.empty()) {
request_cache_control_ = RequestCacheControl(cache_control);
Expand Down
47 changes: 47 additions & 0 deletions source/extensions/filters/http/cache/inline_headers_handles.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
#pragma once

#include "common/http/headers.h"

namespace Envoy {
namespace Extensions {
namespace HttpFilters {
namespace Cache {

inline Http::RegisterCustomInlineHeader<Http::CustomInlineHeaderRegistry::Type::RequestHeaders>
authorization_handle(Http::CustomHeaders::get().Authorization);

inline Http::RegisterCustomInlineHeader<Http::CustomInlineHeaderRegistry::Type::RequestHeaders>
pragma_handle(Http::CustomHeaders::get().Pragma);

inline Http::RegisterCustomInlineHeader<Http::CustomInlineHeaderRegistry::Type::RequestHeaders>
request_cache_control_handle(Http::CustomHeaders::get().CacheControl);

inline Http::RegisterCustomInlineHeader<Http::CustomInlineHeaderRegistry::Type::RequestHeaders>
if_match_handle(Http::CustomHeaders::get().IfMatch);

inline Http::RegisterCustomInlineHeader<Http::CustomInlineHeaderRegistry::Type::RequestHeaders>
if_none_match_handle(Http::CustomHeaders::get().IfNoneMatch);

inline Http::RegisterCustomInlineHeader<Http::CustomInlineHeaderRegistry::Type::RequestHeaders>
if_modified_since_handle(Http::CustomHeaders::get().IfModifiedSince);

inline Http::RegisterCustomInlineHeader<Http::CustomInlineHeaderRegistry::Type::RequestHeaders>
if_unmodified_since_handle(Http::CustomHeaders::get().IfUnmodifiedSince);

inline Http::RegisterCustomInlineHeader<Http::CustomInlineHeaderRegistry::Type::RequestHeaders>
if_range_handle(Http::CustomHeaders::get().IfRange);

// Response headers inline handles
inline Http::RegisterCustomInlineHeader<Http::CustomInlineHeaderRegistry::Type::ResponseHeaders>
response_cache_control_handle(Http::CustomHeaders::get().CacheControl);

inline Http::RegisterCustomInlineHeader<Http::CustomInlineHeaderRegistry::Type::ResponseHeaders>
last_modified_handle(Http::CustomHeaders::get().LastModified);

inline Http::RegisterCustomInlineHeader<Http::CustomInlineHeaderRegistry::Type::ResponseHeaders>
etag_handle(Http::CustomHeaders::get().Etag);

} // namespace Cache
} // namespace HttpFilters
} // namespace Extensions
} // namespace Envoy

0 comments on commit a7e7bd1

Please sign in to comment.