Skip to content

Added cache control to readme files #1924

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 25, 2019
Merged

Added cache control to readme files #1924

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6a0d5f1
Some first steps
hbina Nov 24, 2019
311cafd
Updated README cache-header
hbina Nov 25, 2019
f82d6e3
Applied the same cache-control to render-readme binary
hbina Nov 25, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions src/bin/render-readmes.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,10 @@ use chrono::{TimeZone, Utc};
use diesel::{dsl::any, prelude::*};
use docopt::Docopt;
use flate2::read::GzDecoder;
use reqwest::Client;
use reqwest::{header, Client};
use tar::{self, Archive};

const CACHE_CONTROL_README: &str = "public,max-age=604800";
const DEFAULT_PAGE_SIZE: usize = 25;
const USAGE: &str = "
Usage: render-readmes [options]
Expand Down Expand Up @@ -129,6 +130,8 @@ fn main() {
let content_length = readme.len() as u64;
let content = std::io::Cursor::new(readme);
let readme_path = format!("readmes/{0}/{0}-{1}.html", krate_name, version.num);
let mut extra_headers = header::HeaderMap::new();
extra_headers.insert(header::CACHE_CONTROL, CACHE_CONTROL_README.parse().unwrap());
config
.uploader
.upload(
Expand All @@ -137,7 +140,7 @@ fn main() {
content,
content_length,
"text/html",
None,
extra_headers,
)
.unwrap_or_else(|_| {
panic!(
Expand Down
4 changes: 2 additions & 2 deletions src/s3/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ impl Bucket {
content: R,
content_length: u64,
content_type: &str,
extra_headers: Option<header::HeaderMap>,
extra_headers: header::HeaderMap,
) -> reqwest::Result<reqwest::Response> {
let path = if path.starts_with('/') {
&path[1..]
Expand All @@ -61,7 +61,7 @@ impl Bucket {
.header(header::AUTHORIZATION, auth)
.header(header::CONTENT_TYPE, content_type)
.header(header::DATE, date)
.headers(extra_headers.unwrap_or_else(header::HeaderMap::new))
.headers(extra_headers)
.body(reqwest::Body::sized(content, content_length))
.send()?
.error_for_status()
Expand Down
4 changes: 2 additions & 2 deletions src/tasks/dump_db.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::{
};

use crate::{background_jobs::Environment, uploaders::Uploader, util::errors::std_error_no_send};

use reqwest::header;
use swirl::PerformError;

/// Create CSV dumps of the public information in the database, wrap them in a
Expand Down Expand Up @@ -157,7 +157,7 @@ impl DumpTarball {
tarfile,
content_length,
"application/gzip",
None,
header::HeaderMap::new(),
)
.map_err(std_error_no_send)?;
Ok(())
Expand Down
19 changes: 11 additions & 8 deletions src/uploaders.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ use std::sync::Arc;
use crate::middleware::app::RequestApp;
use crate::models::Crate;

pub const CACHE_CONTROL_IMMUTABLE: &str = "public,max-age=31536000,immutable";
const CACHE_CONTROL_IMMUTABLE: &str = "public,max-age=31536000,immutable";
const CACHE_CONTROL_README: &str = "public,max-age=604800";

#[derive(Clone, Debug)]
pub enum Uploader {
Expand Down Expand Up @@ -94,7 +95,7 @@ impl Uploader {
mut content: R,
content_length: u64,
content_type: &str,
extra_headers: Option<header::HeaderMap>,
extra_headers: header::HeaderMap,
) -> CargoResult<Option<String>> {
match *self {
Uploader::S3 { ref bucket, .. } => {
Expand Down Expand Up @@ -138,17 +139,14 @@ impl Uploader {
let content_length = body.len() as u64;
let content = Cursor::new(body);
let mut extra_headers = header::HeaderMap::new();
extra_headers.insert(
header::CACHE_CONTROL,
CACHE_CONTROL_IMMUTABLE.parse().unwrap(),
);
extra_headers.insert(header::CACHE_CONTROL, CACHE_CONTROL_README.parse().unwrap());
self.upload(
app.http_client(),
&path,
content,
content_length,
"application/x-tar",
Some(extra_headers),
extra_headers,
)?;
Ok(checksum)
}
Expand All @@ -163,13 +161,18 @@ impl Uploader {
let path = Uploader::readme_path(crate_name, vers);
let content_length = readme.len() as u64;
let content = Cursor::new(readme);
let mut extra_headers = header::HeaderMap::new();
extra_headers.insert(
header::CACHE_CONTROL,
CACHE_CONTROL_IMMUTABLE.parse().unwrap(),
Copy link
Contributor

Choose a reason for hiding this comment

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

The variable CACHE_CONTROL_IMMUTABLE is set to public,max-age=31536000,immutable, i.e. the files are tagged to never change and to live for one year in the cache. However, the readme files might change, so let's cache them only for a week, and let's not tag them as immutable. (You may want to read the documentation of the cache-control header).

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 see, sorry for misunderstanding the original Issue. So I assume the only field that is required is max-age=<seconds>?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can keep public as well. Not sure whether it has any effect when setting max-age at the same time, but it certainly doesn't hurt.

);
self.upload(
http_client,
&path,
content,
content_length,
"text/html",
None,
extra_headers,
)?;
Ok(())
}
Expand Down