mirror of
https://github.com/modrinth/code.git
synced 2026-07-31 21:26:40 +00:00
fix: file attribution scanning not marking completed files (#6500)
* fix: file attribution scanning not marking completed files * Clean up unused attribution files/groups on deletion * prepare * Handle file scan failures gracefully with tracking
This commit is contained in:
Generated
+16
@@ -0,0 +1,16 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "\n update file_scans\n set\n attributions_scanned_at = $2,\n attributions_scan_result = $3\n where file_id = $1\n ",
|
||||
"describe": {
|
||||
"columns": [],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Int8",
|
||||
"Timestamptz",
|
||||
"Jsonb"
|
||||
]
|
||||
},
|
||||
"nullable": []
|
||||
},
|
||||
"hash": "11228045ad657e66693d4b384beacbd50d74251f59c1147e71b0ef293a19805e"
|
||||
}
|
||||
Generated
-34
@@ -1,34 +0,0 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "\n SELECT mel.id, mel.flame_project_id, mel.status status\n FROM moderation_external_licenses mel\n WHERE mel.flame_project_id = ANY($1)\n ",
|
||||
"describe": {
|
||||
"columns": [
|
||||
{
|
||||
"ordinal": 0,
|
||||
"name": "id",
|
||||
"type_info": "Int8"
|
||||
},
|
||||
{
|
||||
"ordinal": 1,
|
||||
"name": "flame_project_id",
|
||||
"type_info": "Int4"
|
||||
},
|
||||
{
|
||||
"ordinal": 2,
|
||||
"name": "status",
|
||||
"type_info": "Text"
|
||||
}
|
||||
],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Int4Array"
|
||||
]
|
||||
},
|
||||
"nullable": [
|
||||
false,
|
||||
true,
|
||||
false
|
||||
]
|
||||
},
|
||||
"hash": "3151ef71738a1f0d097aa14967d7b9eb1f24d4de1f81b80c4bd186427edc1399"
|
||||
}
|
||||
Generated
-15
@@ -1,15 +0,0 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "\n update file_scans\n set attributions_scanned_at = $2\n where file_id = $1\n ",
|
||||
"describe": {
|
||||
"columns": [],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Int8",
|
||||
"Timestamptz"
|
||||
]
|
||||
},
|
||||
"nullable": []
|
||||
},
|
||||
"hash": "4748e425c6bbd4d154bf658bf695661485a9700bffa413c5e94d80a525b53e27"
|
||||
}
|
||||
Generated
-28
@@ -1,28 +0,0 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "\n SELECT encode(mef.sha1, 'escape') sha1, mel.status status\n FROM moderation_external_files mef\n INNER JOIN moderation_external_licenses mel ON mef.external_license_id = mel.id\n WHERE mef.sha1 = ANY($1)\n ",
|
||||
"describe": {
|
||||
"columns": [
|
||||
{
|
||||
"ordinal": 0,
|
||||
"name": "sha1",
|
||||
"type_info": "Text"
|
||||
},
|
||||
{
|
||||
"ordinal": 1,
|
||||
"name": "status",
|
||||
"type_info": "Text"
|
||||
}
|
||||
],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"ByteaArray"
|
||||
]
|
||||
},
|
||||
"nullable": [
|
||||
null,
|
||||
false
|
||||
]
|
||||
},
|
||||
"hash": "4cb9fe3dbb2cbfe30a49487f896fb7890f726af2ff11da53f450a88c3dc5fc64"
|
||||
}
|
||||
Generated
-14
@@ -1,14 +0,0 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "\n insert into file_scans (file_id, attributions_scanned_at)\n values ($1, now())\n on conflict (file_id) do update set attributions_scanned_at = now()\n ",
|
||||
"describe": {
|
||||
"columns": [],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Int8"
|
||||
]
|
||||
},
|
||||
"nullable": []
|
||||
},
|
||||
"hash": "80751dbca09a9dcb469a30fe5b8225e520baf50750a78875f582349610439a55"
|
||||
}
|
||||
Generated
+15
@@ -0,0 +1,15 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "\n insert into file_scans (\n file_id,\n attributions_scanned_at,\n attributions_scan_result\n )\n values ($1, now(), $2)\n on conflict (file_id) do update set\n attributions_scanned_at = now(),\n attributions_scan_result = $2\n ",
|
||||
"describe": {
|
||||
"columns": [],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Int8",
|
||||
"Jsonb"
|
||||
]
|
||||
},
|
||||
"nullable": []
|
||||
},
|
||||
"hash": "89e0688880584ca6745961772445cede934143b33d5ca86b6c6f889e5beee543"
|
||||
}
|
||||
+2
-2
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "\n DELETE FROM project_attribution_groups g\n WHERE NOT EXISTS (\n SELECT 1\n FROM project_attribution_files paf\n INNER JOIN override_file_sources ofs ON ofs.sha1 = paf.sha1\n WHERE paf.group_id = g.id\n )\n ",
|
||||
"query": "\n DELETE FROM project_attribution_groups g\n WHERE NOT EXISTS (\n SELECT 1\n FROM project_attribution_files paf\n WHERE paf.group_id = g.id\n )\n ",
|
||||
"describe": {
|
||||
"columns": [],
|
||||
"parameters": {
|
||||
@@ -8,5 +8,5 @@
|
||||
},
|
||||
"nullable": []
|
||||
},
|
||||
"hash": "301959b1ec20a4925f86188bc5e1c0cdd11f0d15004ce9f91fbeefcc3a27f8f4"
|
||||
"hash": "bba6df9227000db16828d99517d1cafb5b682b3a50ea5f31f580a8198023a316"
|
||||
}
|
||||
Generated
-15
@@ -1,15 +0,0 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "\n UPDATE files\n SET metadata = $1\n WHERE id = $2\n ",
|
||||
"describe": {
|
||||
"columns": [],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Jsonb",
|
||||
"Int8"
|
||||
]
|
||||
},
|
||||
"nullable": []
|
||||
},
|
||||
"hash": "c2924fff035e92f7bd2279517310ba391ced72b38be97d462cdfe60048e947db"
|
||||
}
|
||||
Generated
+20
@@ -0,0 +1,20 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "\n select count(*) as \"count!\"\n from file_scans fa\n inner join files f on f.id = fa.file_id\n inner join attribution_enforced_versions aev on aev.id = f.version_id\n where fa.attributions_scanned_at is null\n ",
|
||||
"describe": {
|
||||
"columns": [
|
||||
{
|
||||
"ordinal": 0,
|
||||
"name": "count!",
|
||||
"type_info": "Int8"
|
||||
}
|
||||
],
|
||||
"parameters": {
|
||||
"Left": []
|
||||
},
|
||||
"nullable": [
|
||||
null
|
||||
]
|
||||
},
|
||||
"hash": "c47f6e360d4771339345c48f1eb62b36241639858e1f8e6aa72a2db6b6def51e"
|
||||
}
|
||||
Generated
-15
@@ -1,15 +0,0 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "\n UPDATE files\n SET metadata = $1\n WHERE id = $2\n ",
|
||||
"describe": {
|
||||
"columns": [],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Jsonb",
|
||||
"Int8"
|
||||
]
|
||||
},
|
||||
"nullable": []
|
||||
},
|
||||
"hash": "ccf57f9c1026927afc940a20ebad9fb58ded7171b21e91973d1f13c91eab9b37"
|
||||
}
|
||||
Generated
+12
@@ -0,0 +1,12 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "\n DELETE FROM project_attribution_files paf\n USING project_attribution_groups pag\n WHERE pag.id = paf.group_id\n AND NOT EXISTS (\n SELECT 1\n FROM override_file_sources ofs\n INNER JOIN files f ON f.id = ofs.file_id\n INNER JOIN versions v ON v.id = f.version_id\n WHERE ofs.sha1 = paf.sha1\n AND v.mod_id = pag.project_id\n )\n ",
|
||||
"describe": {
|
||||
"columns": [],
|
||||
"parameters": {
|
||||
"Left": []
|
||||
},
|
||||
"nullable": []
|
||||
},
|
||||
"hash": "d46c53877e97d54f38895f070bdc0a2417ba87ecac65edd03e8dc7a677f5efa9"
|
||||
}
|
||||
Generated
-20
@@ -1,20 +0,0 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "\n select count(*) as \"count!\" from file_scans\n where attributions_scanned_at is null\n ",
|
||||
"describe": {
|
||||
"columns": [
|
||||
{
|
||||
"ordinal": 0,
|
||||
"name": "count!",
|
||||
"type_info": "Int8"
|
||||
}
|
||||
],
|
||||
"parameters": {
|
||||
"Left": []
|
||||
},
|
||||
"nullable": [
|
||||
null
|
||||
]
|
||||
},
|
||||
"hash": "eaafc6b2e29562112ae323e68955aa99e70f8ecbdc1b2500f7d7e89af8fc6e29"
|
||||
}
|
||||
@@ -0,0 +1,2 @@
|
||||
alter table file_scans
|
||||
add column attributions_scan_result jsonb;
|
||||
@@ -25,16 +25,33 @@ use tracing::error;
|
||||
pub const VERSIONS_NAMESPACE: &str = "versions";
|
||||
const VERSION_FILES_NAMESPACE: &str = "versions_files";
|
||||
|
||||
pub async fn cleanup_empty_attribution_groups(
|
||||
pub async fn cleanup_unused_attribution_files_and_groups(
|
||||
transaction: &mut PgTransaction<'_>,
|
||||
) -> Result<(), DatabaseError> {
|
||||
sqlx::query!(
|
||||
"
|
||||
DELETE FROM project_attribution_files paf
|
||||
USING project_attribution_groups pag
|
||||
WHERE pag.id = paf.group_id
|
||||
AND NOT EXISTS (
|
||||
SELECT 1
|
||||
FROM override_file_sources ofs
|
||||
INNER JOIN files f ON f.id = ofs.file_id
|
||||
INNER JOIN versions v ON v.id = f.version_id
|
||||
WHERE ofs.sha1 = paf.sha1
|
||||
AND v.mod_id = pag.project_id
|
||||
)
|
||||
",
|
||||
)
|
||||
.execute(&mut *transaction)
|
||||
.await?;
|
||||
|
||||
sqlx::query!(
|
||||
"
|
||||
DELETE FROM project_attribution_groups g
|
||||
WHERE NOT EXISTS (
|
||||
SELECT 1
|
||||
FROM project_attribution_files paf
|
||||
INNER JOIN override_file_sources ofs ON ofs.sha1 = paf.sha1
|
||||
WHERE paf.group_id = g.id
|
||||
)
|
||||
",
|
||||
@@ -493,7 +510,7 @@ impl DBVersion {
|
||||
.execute(&mut *transaction)
|
||||
.await?;
|
||||
|
||||
cleanup_empty_attribution_groups(transaction).await?;
|
||||
cleanup_unused_attribution_files_and_groups(transaction).await?;
|
||||
|
||||
// Sync dependencies
|
||||
|
||||
|
||||
@@ -19,6 +19,7 @@ use crate::database::models::{DBFileId, DBUserId, DBVersion};
|
||||
use crate::database::{PgPool, PgTransaction, redis::RedisPool};
|
||||
use crate::env::ENV;
|
||||
use crate::file_hosting::{FileHost, FileHostPublicity};
|
||||
use crate::models::error::ApiError;
|
||||
use crate::models::ids::FileId;
|
||||
use crate::models::projects::{
|
||||
AttributionResolution, AttributionResolutionKind, DependencyAttribution,
|
||||
@@ -32,6 +33,8 @@ use crate::util::http::HTTP_CLIENT;
|
||||
|
||||
const PENDING_FILE_SCAN_BATCH_SIZE: i64 = 100;
|
||||
|
||||
type FileScanResult<'a> = std::result::Result<(), ApiError<'a>>;
|
||||
|
||||
#[derive(Clone)]
|
||||
struct PendingFileScan {
|
||||
file_id: DBFileId,
|
||||
@@ -58,8 +61,11 @@ pub async fn scan_all_pending_files(
|
||||
|
||||
let total_to_scan = sqlx::query_scalar!(
|
||||
r#"
|
||||
select count(*) as "count!" from file_scans
|
||||
where attributions_scanned_at is null
|
||||
select count(*) as "count!"
|
||||
from file_scans fa
|
||||
inner join files f on f.id = fa.file_id
|
||||
inner join attribution_enforced_versions aev on aev.id = f.version_id
|
||||
where fa.attributions_scanned_at is null
|
||||
"#,
|
||||
)
|
||||
.fetch_one(db)
|
||||
@@ -113,9 +119,10 @@ async fn scan_pending_files_batch(
|
||||
.await
|
||||
.wrap_err("fetching files to scan")?;
|
||||
|
||||
let fetched_count = files_to_scan.len();
|
||||
|
||||
info!(
|
||||
"Found {} pending files to scan, splitting into jobs of {PENDING_FILE_SCAN_BATCH_SIZE}",
|
||||
files_to_scan.len(),
|
||||
"Found {fetched_count} pending files to scan, splitting into jobs of {PENDING_FILE_SCAN_BATCH_SIZE}",
|
||||
);
|
||||
|
||||
let files_to_scan: Vec<_> = files_to_scan
|
||||
@@ -140,11 +147,26 @@ async fn scan_pending_files_batch(
|
||||
}
|
||||
|
||||
let mut scanned_count = 0;
|
||||
let mut first_err = None;
|
||||
for task in tasks {
|
||||
scanned_count += task
|
||||
.await
|
||||
.wrap_err("joining file scan task")?
|
||||
.wrap_err("scanning pending file chunk")?;
|
||||
match task.await.wrap_err("joining file scan task")? {
|
||||
Ok(count) => scanned_count += count,
|
||||
Err(err) => {
|
||||
if first_err.is_none() {
|
||||
first_err = Some(err);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if let Some(err) = first_err {
|
||||
return Err(err).wrap_err("scanning pending file chunk");
|
||||
}
|
||||
|
||||
if fetched_count > 0 && scanned_count == 0 {
|
||||
return Err(eyre!(
|
||||
"file scan batch made no progress after fetching {fetched_count} files"
|
||||
));
|
||||
}
|
||||
|
||||
info!("Marked {} files as scanned", scanned_count);
|
||||
@@ -164,10 +186,10 @@ async fn scan_pending_files_chunk(
|
||||
for row in files_to_scan {
|
||||
let human_file_id = FileId::from(row.file_id);
|
||||
let span = info_span!("scan", file_id = %human_file_id);
|
||||
async {
|
||||
info!("Scanning file");
|
||||
|
||||
let file_id = row.file_id;
|
||||
let file_id = row.file_id;
|
||||
let result = async {
|
||||
info!("Scanning file");
|
||||
|
||||
let overrides = extract_override_files_from_storage(
|
||||
file_host, file_id, &row.url,
|
||||
@@ -179,6 +201,7 @@ async fn scan_pending_files_chunk(
|
||||
|
||||
if overrides.is_empty() {
|
||||
info!("Found no overrides");
|
||||
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
@@ -209,20 +232,6 @@ async fn scan_pending_files_chunk(
|
||||
eyre!("persisting attribution results for file {file_id:?}")
|
||||
})?;
|
||||
|
||||
let now = Utc::now();
|
||||
sqlx::query!(
|
||||
"
|
||||
update file_scans
|
||||
set attributions_scanned_at = $2
|
||||
where file_id = $1
|
||||
",
|
||||
file_id.0,
|
||||
now,
|
||||
)
|
||||
.execute(&mut txn)
|
||||
.await
|
||||
.wrap_err("marking file as scanned")?;
|
||||
|
||||
txn.commit()
|
||||
.await
|
||||
.wrap_err("committing file scan transaction")?;
|
||||
@@ -230,7 +239,21 @@ async fn scan_pending_files_chunk(
|
||||
eyre::Ok(())
|
||||
}
|
||||
.instrument(span)
|
||||
.await?;
|
||||
.await;
|
||||
|
||||
let scan_result = file_scan_result(&result);
|
||||
match result {
|
||||
Ok(()) => {
|
||||
info!(%human_file_id, "Successfully scanned file");
|
||||
}
|
||||
Err(err) => {
|
||||
warn!(%human_file_id, "Failed to scan file: {err:?}");
|
||||
}
|
||||
}
|
||||
|
||||
update_file_scan_result(db, file_id, scan_result)
|
||||
.await
|
||||
.wrap_err("marking file as scanned")?;
|
||||
|
||||
scanned_count += 1;
|
||||
}
|
||||
@@ -245,6 +268,25 @@ pub async fn scan_file(
|
||||
project_id: DBProjectId,
|
||||
file_id: DBFileId,
|
||||
file_url: &str,
|
||||
) -> Result<()> {
|
||||
let result =
|
||||
scan_file_inner(txn, redis, file_host, project_id, file_id, file_url)
|
||||
.await;
|
||||
|
||||
upsert_file_scan_result(txn, file_id, file_scan_result(&result))
|
||||
.await
|
||||
.wrap_err("marking file as scanned")?;
|
||||
|
||||
result
|
||||
}
|
||||
|
||||
async fn scan_file_inner(
|
||||
txn: &mut PgTransaction<'_>,
|
||||
redis: &RedisPool,
|
||||
file_host: &dyn FileHost,
|
||||
project_id: DBProjectId,
|
||||
file_id: DBFileId,
|
||||
file_url: &str,
|
||||
) -> Result<()> {
|
||||
let overrides =
|
||||
extract_override_files_from_storage(file_host, file_id, file_url)
|
||||
@@ -269,17 +311,69 @@ pub async fn scan_file(
|
||||
})?;
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn file_scan_result(result: &Result<()>) -> FileScanResult<'static> {
|
||||
match result {
|
||||
Ok(()) => Ok(()),
|
||||
Err(err) => Err(ApiError {
|
||||
error: "internal_error",
|
||||
description: format!("{err:#}"),
|
||||
details: None,
|
||||
}),
|
||||
}
|
||||
}
|
||||
|
||||
async fn update_file_scan_result(
|
||||
db: &PgPool,
|
||||
file_id: DBFileId,
|
||||
result: FileScanResult<'_>,
|
||||
) -> Result<()> {
|
||||
let now = Utc::now();
|
||||
|
||||
sqlx::query!(
|
||||
"
|
||||
insert into file_scans (file_id, attributions_scanned_at)
|
||||
values ($1, now())
|
||||
on conflict (file_id) do update set attributions_scanned_at = now()
|
||||
",
|
||||
r#"
|
||||
update file_scans
|
||||
set
|
||||
attributions_scanned_at = $2,
|
||||
attributions_scan_result = $3
|
||||
where file_id = $1
|
||||
"#,
|
||||
file_id.0,
|
||||
now,
|
||||
sqlx::types::Json(result) as _,
|
||||
)
|
||||
.execute(db)
|
||||
.await
|
||||
.wrap_err("updating file scan result")?;
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn upsert_file_scan_result(
|
||||
txn: &mut PgTransaction<'_>,
|
||||
file_id: DBFileId,
|
||||
result: FileScanResult<'_>,
|
||||
) -> Result<()> {
|
||||
sqlx::query!(
|
||||
r#"
|
||||
insert into file_scans (
|
||||
file_id,
|
||||
attributions_scanned_at,
|
||||
attributions_scan_result
|
||||
)
|
||||
values ($1, now(), $2)
|
||||
on conflict (file_id) do update set
|
||||
attributions_scanned_at = now(),
|
||||
attributions_scan_result = $2
|
||||
"#,
|
||||
file_id.0,
|
||||
sqlx::types::Json(result) as _,
|
||||
)
|
||||
.execute(&mut *txn)
|
||||
.await
|
||||
.wrap_err("marking file as scanned")?;
|
||||
.wrap_err("upserting file scan result")?;
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -722,10 +722,8 @@ pub async fn delete_file(
|
||||
.execute(&mut transaction)
|
||||
.await?;
|
||||
|
||||
database::models::version_item::cleanup_empty_attribution_groups(
|
||||
&mut transaction,
|
||||
)
|
||||
.await?;
|
||||
database::models::version_item::cleanup_unused_attribution_files_and_groups(&mut transaction)
|
||||
.await?;
|
||||
|
||||
delphi::send_tech_review_exit_file_deleted_message_if_exited(
|
||||
row.project_id,
|
||||
|
||||
Reference in New Issue
Block a user