mirror of https://github.com/lldap/lldap.git
server: Fix panic due to database collation
When the database's collation is not "C", the DB order is not the same as the Rust order. As such, asserting that the elements are in increasing order fails. However, since both queries get the order from the database, they should be in the same order. With too many users, the query had a giant filter `IN (u1, u2, u3, ...)`. In PostgreSQL, we can pass the users as an array instead, but that doesn't work with SQLite. Instead, we repeat the filter from the previous query to get the same users/groups, as a subquery.
This commit is contained in:
parent
8f2391a792
commit
1c65cd115e
|
@ -79,25 +79,24 @@ fn get_group_filter_expr(filter: GroupRequestFilter) -> Cond {
|
|||
impl GroupListerBackendHandler for SqlBackendHandler {
|
||||
#[instrument(skip(self), level = "debug", ret, err)]
|
||||
async fn list_groups(&self, filters: Option<GroupRequestFilter>) -> Result<Vec<Group>> {
|
||||
let filters = filters
|
||||
.map(|f| {
|
||||
GroupColumn::GroupId
|
||||
.in_subquery(
|
||||
model::Group::find()
|
||||
.find_also_linked(model::memberships::GroupToUser)
|
||||
.select_only()
|
||||
.column(GroupColumn::GroupId)
|
||||
.filter(get_group_filter_expr(f))
|
||||
.into_query(),
|
||||
)
|
||||
.into_condition()
|
||||
})
|
||||
.unwrap_or_else(|| SimpleExpr::Value(true.into()).into_condition());
|
||||
let results = model::Group::find()
|
||||
.order_by_asc(GroupColumn::GroupId)
|
||||
.find_with_related(model::Membership)
|
||||
.filter(
|
||||
filters
|
||||
.map(|f| {
|
||||
GroupColumn::GroupId
|
||||
.in_subquery(
|
||||
model::Group::find()
|
||||
.find_also_linked(model::memberships::GroupToUser)
|
||||
.select_only()
|
||||
.column(GroupColumn::GroupId)
|
||||
.filter(get_group_filter_expr(f))
|
||||
.into_query(),
|
||||
)
|
||||
.into_condition()
|
||||
})
|
||||
.unwrap_or_else(|| SimpleExpr::Value(true.into()).into_condition()),
|
||||
)
|
||||
.filter(filters.clone())
|
||||
.all(&self.sql_pool)
|
||||
.await?;
|
||||
let mut groups: Vec<_> = results
|
||||
|
@ -110,9 +109,16 @@ impl GroupListerBackendHandler for SqlBackendHandler {
|
|||
}
|
||||
})
|
||||
.collect();
|
||||
let group_ids = groups.iter().map(|u| &u.id);
|
||||
let attributes = model::GroupAttributes::find()
|
||||
.filter(model::GroupAttributesColumn::GroupId.is_in(group_ids))
|
||||
.filter(
|
||||
model::GroupAttributesColumn::GroupId.in_subquery(
|
||||
model::Group::find()
|
||||
.filter(filters)
|
||||
.select_only()
|
||||
.column(model::groups::Column::GroupId)
|
||||
.into_query(),
|
||||
),
|
||||
)
|
||||
.order_by_asc(model::GroupAttributesColumn::GroupId)
|
||||
.order_by_asc(model::GroupAttributesColumn::AttributeName)
|
||||
.all(&self.sql_pool)
|
||||
|
@ -120,12 +126,6 @@ impl GroupListerBackendHandler for SqlBackendHandler {
|
|||
let mut attributes_iter = attributes.into_iter().peekable();
|
||||
use itertools::Itertools; // For take_while_ref
|
||||
for group in groups.iter_mut() {
|
||||
assert!(attributes_iter
|
||||
.peek()
|
||||
.map(|u| u.group_id >= group.id)
|
||||
.unwrap_or(true),
|
||||
"Attributes are not sorted, groups are not sorted, or previous group didn't consume all the attributes");
|
||||
|
||||
group.attributes = attributes_iter
|
||||
.take_while_ref(|u| u.group_id == group.id)
|
||||
.map(AttributeValue::from)
|
||||
|
|
|
@ -104,23 +104,22 @@ impl UserListerBackendHandler for SqlBackendHandler {
|
|||
// To simplify the query, we always fetch groups. TODO: cleanup.
|
||||
_get_groups: bool,
|
||||
) -> Result<Vec<UserAndGroups>> {
|
||||
let filters = filters
|
||||
.map(|f| {
|
||||
UserColumn::UserId
|
||||
.in_subquery(
|
||||
model::User::find()
|
||||
.find_also_linked(model::memberships::UserToGroup)
|
||||
.select_only()
|
||||
.column(UserColumn::UserId)
|
||||
.filter(get_user_filter_expr(f))
|
||||
.into_query(),
|
||||
)
|
||||
.into_condition()
|
||||
})
|
||||
.unwrap_or_else(|| SimpleExpr::Value(true.into()).into_condition());
|
||||
let mut users: Vec<_> = model::User::find()
|
||||
.filter(
|
||||
filters
|
||||
.map(|f| {
|
||||
UserColumn::UserId
|
||||
.in_subquery(
|
||||
model::User::find()
|
||||
.find_also_linked(model::memberships::UserToGroup)
|
||||
.select_only()
|
||||
.column(UserColumn::UserId)
|
||||
.filter(get_user_filter_expr(f))
|
||||
.into_query(),
|
||||
)
|
||||
.into_condition()
|
||||
})
|
||||
.unwrap_or_else(|| SimpleExpr::Value(true.into()).into_condition()),
|
||||
)
|
||||
.filter(filters.clone())
|
||||
.order_by_asc(UserColumn::UserId)
|
||||
.find_with_linked(model::memberships::UserToGroup)
|
||||
.order_by_asc(SimpleExpr::Column(
|
||||
|
@ -134,10 +133,18 @@ impl UserListerBackendHandler for SqlBackendHandler {
|
|||
groups: Some(groups.into_iter().map(Into::<GroupDetails>::into).collect()),
|
||||
})
|
||||
.collect();
|
||||
|
||||
// At this point, the users don't have attributes, we need to populate it with another query.
|
||||
let user_ids = users.iter().map(|u| &u.user.user_id);
|
||||
let attributes = model::UserAttributes::find()
|
||||
.filter(model::UserAttributesColumn::UserId.is_in(user_ids))
|
||||
.filter(
|
||||
model::UserAttributesColumn::UserId.in_subquery(
|
||||
model::User::find()
|
||||
.filter(filters)
|
||||
.select_only()
|
||||
.column(model::users::Column::UserId)
|
||||
.into_query(),
|
||||
),
|
||||
)
|
||||
.order_by_asc(model::UserAttributesColumn::UserId)
|
||||
.order_by_asc(model::UserAttributesColumn::AttributeName)
|
||||
.all(&self.sql_pool)
|
||||
|
@ -145,12 +152,6 @@ impl UserListerBackendHandler for SqlBackendHandler {
|
|||
let mut attributes_iter = attributes.into_iter().peekable();
|
||||
use itertools::Itertools; // For take_while_ref
|
||||
for user in users.iter_mut() {
|
||||
assert!(attributes_iter
|
||||
.peek()
|
||||
.map(|u| u.user_id >= user.user.user_id)
|
||||
.unwrap_or(true),
|
||||
"Attributes are not sorted, users are not sorted, or previous user didn't consume all the attributes");
|
||||
|
||||
user.user.attributes = attributes_iter
|
||||
.take_while_ref(|u| u.user_id == user.user.user_id)
|
||||
.map(AttributeValue::from)
|
||||
|
|
Loading…
Reference in New Issue