diff --git a/apps/labrinth/src/routes/v3/projects.rs b/apps/labrinth/src/routes/v3/projects.rs index 18afc646e6..112ed1e438 100644 --- a/apps/labrinth/src/routes/v3/projects.rs +++ b/apps/labrinth/src/routes/v3/projects.rs @@ -1102,7 +1102,8 @@ pub async fn project_edit_internal( Ok(true) } - let mut reindex_versions = false; + let mut reindex_versions = new_project.categories.is_some() + || new_project.additional_categories.is_some(); reindex_versions |= update( &mut transaction, @@ -1670,7 +1671,7 @@ pub async fn projects_edit( }; } - bulk_edit_project_categories( + let mut reindex_versions = bulk_edit_project_categories( &categories, &project.categories, project.inner.id as db_ids::DBProjectId, @@ -1685,7 +1686,7 @@ pub async fn projects_edit( ) .await?; - bulk_edit_project_categories( + reindex_versions |= bulk_edit_project_categories( &categories, &project.additional_categories, project.inner.id as db_ids::DBProjectId, @@ -1744,20 +1745,37 @@ pub async fn projects_edit( } } - changed_projects.push((project.inner.id, project.inner.slug)); + changed_projects.push(( + project.inner.id, + project.inner.slug, + project.versions, + reindex_versions, + )); } transaction.commit().await?; - for (project_id, slug) in changed_projects { - clear_project_cache_and_queue_search( - &redis, - &search_state, - project_id, - slug, - None, - ) - .await?; + for (project_id, slug, versions, reindex_versions) in changed_projects { + if reindex_versions { + db_models::DBProject::clear_cache(project_id, slug, None, &redis) + .await?; + search_state + .queue + .push_version_changes( + project_id.into(), + versions.into_iter().map(VersionId::from), + ) + .await; + } else { + clear_project_cache_and_queue_search( + &redis, + &search_state, + project_id, + slug, + None, + ) + .await?; + } } Ok(HttpResponse::NoContent().body("")) @@ -1771,7 +1789,7 @@ pub async fn bulk_edit_project_categories( max_num_categories: usize, is_additional: bool, transaction: &mut PgTransaction<'_>, -) -> Result<(), ApiError> { +) -> Result { let mut set_categories = if let Some(categories) = bulk_changes.categories.clone() { categories @@ -1798,7 +1816,8 @@ pub async fn bulk_edit_project_categories( } } - if &set_categories != project_categories { + let changed = &set_categories != project_categories; + if changed { sqlx::query!( " DELETE FROM mods_categories @@ -1831,7 +1850,7 @@ pub async fn bulk_edit_project_categories( DBModCategory::insert_many(mod_categories, &mut *transaction).await?; } - Ok(()) + Ok(changed) } #[derive(Serialize, Deserialize)] diff --git a/apps/labrinth/src/search/backend/typesense/filter_rewrite.rs b/apps/labrinth/src/search/backend/typesense/filter_rewrite.rs index 7580fdb38d..e95f4c25cb 100644 --- a/apps/labrinth/src/search/backend/typesense/filter_rewrite.rs +++ b/apps/labrinth/src/search/backend/typesense/filter_rewrite.rs @@ -63,33 +63,12 @@ pub(super) fn rewrite_filter_for_join( eyre!("could not determine filter field in `{expression}`") })?; let mut clause = JoinedFilterClause::default(); - if field == "categories" { - let project_expression = - expression.replacen("categories", "project_categories", 1); - if is_negative_filter(expression) { - clause.project.push(project_expression); - clause.version.push(version_filter_expression(expression)); - Ok(vec![clause]) - } else { - Ok(vec![ - JoinedFilterClause { - project: vec![project_expression], - version: Vec::new(), - }, - JoinedFilterClause { - project: Vec::new(), - version: vec![version_filter_expression(expression)], - }, - ]) - } + if is_version_filter_field(field) { + clause.version.push(version_filter_expression(expression)); } else { - if is_version_filter_field(field) { - clause.version.push(version_filter_expression(expression)); - } else { - clause.project.push(expression.to_string()); - } - Ok(vec![clause]) + clause.project.push(expression.to_string()); } + Ok(vec![clause]) } let clauses = parse(filter)?; @@ -129,12 +108,6 @@ fn version_filter_expression(expression: &str) -> String { format!("{field}:{value}") } -fn is_negative_filter(expression: &str) -> bool { - expression - .split_once(':') - .is_some_and(|(_, value)| value.trim_start().starts_with("!=")) -} - fn filter_field(expression: &str) -> Option<&str> { let operator = expression.find(':')?; let field = expression[..operator].trim(); @@ -413,7 +386,7 @@ mod tests { "versions", ) .unwrap(), - "(project_categories:= fabric && $versions(game_versions: 1.21)) || $versions(categories: fabric && game_versions: 1.21)" + "$versions(categories: fabric && game_versions: 1.21)" ); } @@ -425,7 +398,7 @@ mod tests { "versions", ) .unwrap(), - "(license:= MIT && project_categories:= fabric) || (license:= MIT && $versions(categories: fabric))" + "(license:= MIT && $versions(categories: fabric))" ); } @@ -437,16 +410,16 @@ mod tests { "versions", ) .unwrap(), - "(license:= MIT && $versions(game_versions: 1.21)) || (project_categories:= fabric && $versions(game_versions: 1.21)) || $versions(categories: fabric && game_versions: 1.21)" + "(license:= MIT && $versions(game_versions: 1.21)) || $versions(categories: fabric && game_versions: 1.21)" ); } #[test] - fn negative_categories_require_project_and_version_exclusion() { + fn negative_categories_use_inherited_version_categories() { assert_eq!( rewrite_filter_for_join("categories:!= fabric", "versions") .unwrap(), - "(project_categories:!= fabric && $versions(categories:!= fabric))" + "$versions(categories:!= fabric)" ); } } diff --git a/apps/labrinth/src/search/backend/typesense/mod.rs b/apps/labrinth/src/search/backend/typesense/mod.rs index d893b8e2c0..3a08dc960a 100644 --- a/apps/labrinth/src/search/backend/typesense/mod.rs +++ b/apps/labrinth/src/search/backend/typesense/mod.rs @@ -68,6 +68,11 @@ //! We do also have the `index-search` task which does a full reindex, but this //! shouldn't be required in normal operation. //! +//! Project categories are deliberately denormalized into every version +//! document to avoid a second join when categories are combined with other +//! version filters. Changing a project's categories must therefore reindex all +//! of that project's version documents. +//! //! When batching Typesense update operations, we batch by both project count //! and document count. Since a project can have an unbounded number of //! versions, we can (and will) have projects with thousands of versions. If we @@ -88,6 +93,11 @@ //! //! If you search for `categories = fabric AND game_versions = 1.21`, then //! you're searching for *one* project with *both* these conditions set. +//! Project categories are inherited by every version document, while loaders +//! remain specific to each version. This preserves the legacy behavior where +//! `categories` can match either a project category or a loader, and allows +//! both conditions to use one version join. +//! //! Therefore, this uses *one* join over versions, rather than two joins //! each matching different versions of the same project. Using one join here //! is faster than two. It's even faster if you search for more facets like @@ -100,7 +110,7 @@ //! author), since the full set of categories, environments, etc. is known //! and finite. All of these values can be represented as whitespace-free, //! atomic, single tokens in Typesense, if we set -//! `symbols_to_index = ["=", ".", "_"]`. This means that `fabric-api` is +//! `symbols_to_index = ["-", ".", "_"]`. This means that `fabric-api` is //! treated as one token `fabric-api`, not as `fabric` and `api`. So if we use //! the filter `categories:fabric-api`, it will match for that *one token* //! `fabric-api` instead of full text search. diff --git a/apps/labrinth/src/search/indexing.rs b/apps/labrinth/src/search/indexing.rs index 93ec7f16d0..3dbbb25459 100644 --- a/apps/labrinth/src/search/indexing.rs +++ b/apps/labrinth/src/search/indexing.rs @@ -491,6 +491,9 @@ async fn build_search_documents( } else { (vec![], vec![]) }; + let mut project_categories = categories; + project_categories.sort(); + project_categories.dedup(); let dependencies = dependencies .get(&project.id) .map(|x| x.clone()) @@ -595,7 +598,8 @@ async fn build_search_documents( // These were previously considered the loader, and in v2, the loader is a category for searching. // So to avoid breakage or awkward conversions, we just consider those loader_fields to be categories. // The loaders are kept in the project document's aggregated loader fields as well, so that no information is lost on retrieval. - let mut version_categories = version.loaders.clone(); + let mut version_categories = project_categories.clone(); + version_categories.extend(version.loaders.iter().cloned()); let mrpack_loaders = fields .get("mrpack_loaders") .into_iter() @@ -644,9 +648,6 @@ async fn build_search_documents( }) })); - let mut project_categories = categories; - project_categories.sort(); - project_categories.dedup(); let mut categories = project_categories.clone(); categories.extend(project_loaders.iter().cloned());