Skip to content

Commit 1ad3f9d

Browse files
committed
Explicitly never select categories.path column
This is necessary because: - The path column is of type Ltree - To get text out of an ltree column, it has to be selected using the postgres function ltree2text - We can't modify the select statement generated by the table! macro to select columns with type ltree differently - So we'd have to explicitly select columns for all Category queries anyway - But we never use categories.path, only query against it, so save all this trouble and just never select it in the same way Crate never selects the fulltext search columns. There might be better ways to do this, or at the very least a better way of defining the type aliases, but this is what I got working.
1 parent 6240588 commit 1ad3f9d

File tree

4 files changed

+63
-12
lines changed

4 files changed

+63
-12
lines changed

src/controllers/category.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,7 @@ pub fn index(req: &mut Request) -> CargoResult<Response> {
3737
pub fn show(req: &mut Request) -> CargoResult<Response> {
3838
let slug = &req.params()["category_id"];
3939
let conn = req.db_conn()?;
40-
let cat = categories::table
41-
.filter(categories::slug.eq(::lower(slug)))
40+
let cat = Category::by_slug(slug)
4241
.first::<Category>(&*conn)?;
4342
let subcats = cat.subcategories(&conn)?
4443
.into_iter()

src/controllers/krate/metadata.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ pub fn show(req: &mut Request) -> CargoResult<Response> {
114114
.load(&*conn)?;
115115
let cats = CrateCategory::belonging_to(&krate)
116116
.inner_join(categories::table)
117-
.select(categories::all_columns)
117+
.select(::models::category::ALL_COLUMNS)
118118
.load(&*conn)?;
119119
let recent_downloads = CrateDownload::belonging_to(&krate)
120120
.filter(crate_downloads::date.gt(date(now - 90.days())))

src/models/category.rs

Lines changed: 60 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use chrono::NaiveDateTime;
2-
use diesel::*;
2+
use diesel::{*, self};
33

44
use models::Crate;
55
use schema::*;
@@ -16,6 +16,40 @@ pub struct Category {
1616
pub created_at: NaiveDateTime,
1717
}
1818

19+
/// We literally never want to select `categories.path`
20+
/// so we provide this type and constant to pass to `.select`
21+
type AllColumns = (
22+
categories::id,
23+
categories::category,
24+
categories::slug,
25+
categories::description,
26+
categories::crates_cnt,
27+
categories::created_at,
28+
);
29+
30+
pub const ALL_COLUMNS: AllColumns = (
31+
categories::id,
32+
categories::category,
33+
categories::slug,
34+
categories::description,
35+
categories::crates_cnt,
36+
categories::created_at,
37+
);
38+
39+
type All = diesel::dsl::Select<categories::table, AllColumns>;
40+
type WithSlug<'a> = diesel::dsl::Eq<categories::slug, ::lower::HelperType<&'a str>>;
41+
type BySlug<'a> = diesel::dsl::Filter<All, WithSlug<'a>>;
42+
type WithSlugsCaseSensitive<'a> = diesel::dsl::Eq<
43+
categories::slug,
44+
diesel::pg::expression::array_comparison::Any<
45+
diesel::expression::bound::Bound<
46+
diesel::sql_types::Array<diesel::sql_types::Text>,
47+
&'a [&'a str]
48+
>
49+
>
50+
>;
51+
type BySlugsCaseSensitive<'a> = diesel::dsl::Filter<All, WithSlugsCaseSensitive<'a>>;
52+
1953
#[derive(Associations, Insertable, Identifiable, Debug, Clone, Copy)]
2054
#[belongs_to(Category)]
2155
#[belongs_to(Crate)]
@@ -27,6 +61,27 @@ pub struct CrateCategory {
2761
}
2862

2963
impl Category {
64+
pub fn with_slug(slug: &str) -> WithSlug {
65+
categories::slug.eq(::lower(slug))
66+
}
67+
68+
pub fn by_slug(slug: &str) -> BySlug {
69+
Category::all().filter(Self::with_slug(slug))
70+
}
71+
72+
pub fn with_slugs_case_sensitive<'a>(slugs: &'a [&'a str]) -> WithSlugsCaseSensitive<'a> {
73+
use diesel::dsl::any;
74+
categories::slug.eq(any(slugs))
75+
}
76+
77+
pub fn by_slugs_case_sensitive<'a>(slugs: &'a [&'a str]) -> BySlugsCaseSensitive<'a> {
78+
Category::all().filter(Self::with_slugs_case_sensitive(slugs))
79+
}
80+
81+
pub fn all() -> All {
82+
categories::table.select(ALL_COLUMNS)
83+
}
84+
3085
pub fn encodable(self) -> EncodableCategory {
3186
let Category {
3287
crates_cnt,
@@ -51,11 +106,9 @@ impl Category {
51106
krate: &Crate,
52107
slugs: &[&'a str],
53108
) -> QueryResult<Vec<&'a str>> {
54-
use diesel::dsl::any;
55109

56110
conn.transaction(|| {
57-
let categories = categories::table
58-
.filter(categories::slug.eq(any(slugs)))
111+
let categories = Category::by_slugs_case_sensitive(slugs)
59112
.load::<Category>(conn)?;
60113
let invalid_categories = slugs
61114
.iter()
@@ -121,8 +174,7 @@ impl Category {
121174
/// The intention is to be able to have slugs or parent categories arrayed in order, to
122175
/// offer the frontend, for examples, slugs to create links to each parent category in turn.
123176
pub fn parent_categories(&self, conn: &PgConnection) -> QueryResult<Vec<Category>> {
124-
use diesel::expression::dsl::*;
125-
use diesel::types::Text;
177+
use diesel::sql_types::Text;
126178

127179
sql_query(include_str!("../parent_categories.sql"))
128180
.bind::<Text, _>(&self.slug)
@@ -147,6 +199,7 @@ impl<'a> NewCategory<'a> {
147199
.on_conflict(slug)
148200
.do_update()
149201
.set(self)
202+
.returning(ALL_COLUMNS)
150203
.get_result(conn)
151204
}
152205
}
@@ -370,8 +423,7 @@ mod tests {
370423
.execute(&conn)
371424
.unwrap();
372425

373-
let cat = categories
374-
.filter(slug.eq("cat1::sub1"))
426+
let cat = Category::by_slug("cat1::sub1")
375427
.first::<Category>(&conn)
376428
.unwrap();
377429
let subcats = cat.subcategories(&conn).unwrap();

src/models/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ pub use self::version::{NewVersion, Version};
1717
pub mod helpers;
1818

1919
mod badge;
20-
mod category;
20+
pub mod category;
2121
mod crate_owner_invitation;
2222
pub mod dependency;
2323
mod download;

0 commit comments

Comments
 (0)