Merge #1238

1238: Move `readme_rendered_at` off of the versions table r=sgrif

This column is never used in code other than for filtering/recording
purposes. We do not put it on the `Version` struct, and don't need to be
sending it over the wire every time we query that table.

Author: bors-voyager[bot]

migrations/2018-01-18-174843_create_readme_renderings/down.sql

@@ -0,0 +1,7 @@
+ALTER TABLE versions ADD COLUMN readme_rendered_at TIMESTAMP;
+
+UPDATE versions SET readme_rendered_at = readme_renderings.rendered_at
+  FROM readme_renderings
+  WHERE readme_renderings.version_id = versions.id;
+
+DROP TABLE readme_renderings;

migrations/2018-01-18-174843_create_readme_renderings/up.sql

@@ -0,0 +1,9 @@
+CREATE TABLE readme_renderings (
+  version_id INTEGER NOT NULL PRIMARY KEY REFERENCES versions (id) ON DELETE CASCADE,
+  rendered_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP
+);
+
+INSERT INTO readme_renderings (version_id, rendered_at)
+  SELECT id, readme_rendered_at FROM versions WHERE readme_rendered_at IS NOT NULL;
+
+ALTER TABLE versions DROP COLUMN readme_rendered_at;

src/bin/render-readmes.rs

@@ -79,10 +79,11 @@ fn main() {
 
     let mut query = versions::table
         .inner_join(crates::table)
+        .left_outer_join(readme_renderings::table)
         .filter(
-            versions::readme_rendered_at
+            readme_renderings::rendered_at
                 .lt(older_than)
-                .or(versions::readme_rendered_at.is_null()),
+                .or(readme_renderings::version_id.is_null()),
         )
         .select(versions::id)
         .into_boxed();

src/schema.rs

@@ -527,6 +527,26 @@ table! {
     }
 }
 
+table! {
+    /// Representation of the `readme_renderings` table.
+    ///
+    /// (Automatically generated by Diesel.)
+    readme_renderings (version_id) {
+        /// The `version_id` column of the `readme_renderings` table.
+        ///
+        /// Its SQL type is `Int4`.
+        ///
+        /// (Automatically generated by Diesel.)
+        version_id -> Int4,
+        /// The `rendered_at` column of the `readme_renderings` table.
+        ///
+        /// Its SQL type is `Timestamp`.
+        ///
+        /// (Automatically generated by Diesel.)
+        rendered_at -> Timestamp,
+    }
+}
+
 table! {
     /// Representation of the `reserved_crate_names` table.
     ///
@@ -764,12 +784,6 @@ table! {
         ///
         /// (Automatically generated by Diesel.)
         license -> Nullable<Varchar>,
-        /// The `readme_rendered_at` column of the `versions` table.
-        ///
-        /// Its SQL type is `Nullable<Timestamp>`.
-        ///
-        /// (Automatically generated by Diesel.)
-        readme_rendered_at -> Nullable<Timestamp>,
     }
 }
 
@@ -788,6 +802,7 @@ joinable!(dependencies -> versions (version_id));
 joinable!(emails -> users (user_id));
 joinable!(follows -> crates (crate_id));
 joinable!(follows -> users (user_id));
+joinable!(readme_renderings -> versions (version_id));
 joinable!(version_authors -> users (user_id));
 joinable!(version_authors -> versions (version_id));
 joinable!(version_downloads -> versions (version_id));
@@ -808,6 +823,7 @@ allow_tables_to_appear_in_same_query!(
     follows,
     keywords,
     metadata,
+    readme_renderings,
     reserved_crate_names,
     teams,
     users,

src/tests/version.rs

@@ -93,3 +93,19 @@ fn authors() {
     let json = json.as_object().unwrap();
     assert!(json.contains_key(&"users".to_string()));
 }
+
+#[test]
+fn record_rerendered_readme_time() {
+    let (_b, app, _middle) = ::app();
+    let version = {
+        let conn = app.diesel_database.get().unwrap();
+        let u = ::new_user("foo").create_or_update(&conn).unwrap();
+        let c = ::CrateBuilder::new("foo_authors", u.id).expect_build(&conn);
+        ::new_version(c.id, "1.0.0").save(&conn, &[]).unwrap()
+    };
+    {
+        let conn = app.diesel_database.get().unwrap();
+        version.record_readme_rendering(&conn).unwrap();
+        version.record_readme_rendering(&conn).unwrap();
+    }
+}

src/version/mod.rs

@@ -128,11 +128,14 @@ impl Version {
     }
 
     pub fn record_readme_rendering(&self, conn: &PgConnection) -> QueryResult<usize> {
-        use schema::versions::dsl::readme_rendered_at;
+        use schema::readme_renderings::dsl::*;
         use diesel::dsl::now;
 
-        diesel::update(self)
-            .set(readme_rendered_at.eq(now.nullable()))
+        diesel::insert_into(readme_renderings)
+            .values(version_id.eq(self.id))
+            .on_conflict(version_id)
+            .do_update()
+            .set(rendered_at.eq(now))
             .execute(conn)
     }
 }
@@ -227,7 +230,6 @@ impl Queryable<versions::SqlType, Pg> for Version {
         Option<String>,
         bool,
         Option<String>,
-        Option<NaiveDateTime>,
     );
 
     fn build(row: Self::Row) -> Self {