From 06f0983de103a7c333e320f938439ccca33d322d Mon Sep 17 00:00:00 2001 From: Nikolaus Glombiewski Date: Fri, 28 Jun 2024 15:24:39 +0200 Subject: [PATCH 01/17] Add FAIR dataset deletion and expiration; --- operators/src/error.rs | 4 + services/src/api/handlers/upload.rs | 46 +- ...migration_0010_delete_uploaded_datasets.rs | 24 + services/src/contexts/migrations/mod.rs | 3 + services/src/contexts/mod.rs | 2 +- services/src/datasets/upload.rs | 52 + services/src/error.rs | 8 + services/src/pro/api/apidoc.rs | 8 +- services/src/pro/api/handlers/datasets.rs | 212 ++- .../contexts/migrations/current_schema.sql | 22 + ...migration_0010_delete_uploaded_datasets.rs | 39 + services/src/pro/contexts/migrations/mod.rs | 3 + services/src/pro/contexts/mod.rs | 6 +- services/src/pro/datasets/mod.rs | 5 + services/src/pro/datasets/postgres.rs | 1339 ++++++++++++++++- services/src/pro/datasets/storage.rs | 197 +++ services/src/util/tests.rs | 10 + 17 files changed, 1895 insertions(+), 85 deletions(-) create mode 100644 services/src/contexts/migrations/migration_0010_delete_uploaded_datasets.rs create mode 100644 services/src/pro/contexts/migrations/migration_0010_delete_uploaded_datasets.rs create mode 100644 services/src/pro/datasets/storage.rs diff --git a/operators/src/error.rs b/operators/src/error.rs index 0aa772f42..3ebc2e4bb 100644 --- a/operators/src/error.rs +++ b/operators/src/error.rs @@ -500,6 +500,10 @@ pub enum Error { Bb8Postgres { source: bb8::RunError, }, + + DatasetDeleted { + id: String, + }, } impl From for Error { diff --git a/services/src/api/handlers/upload.rs b/services/src/api/handlers/upload.rs index fc14bb6d8..60d6ad5a4 100644 --- a/services/src/api/handlers/upload.rs +++ b/services/src/api/handlers/upload.rs @@ -1,19 +1,16 @@ use crate::api::model::responses::IdResponse; use crate::contexts::{ApplicationContext, SessionContext}; -use crate::datasets::upload::{FileId, FileUpload, Upload, UploadDb, UploadId, UploadRootPath}; +use crate::datasets::upload::{create_upload, Upload, UploadDb, UploadId, UploadRootPath}; +use crate::error::Error; use crate::error::Result; -use crate::error::{self, Error}; use crate::util::path_with_base_path; use actix_multipart::Multipart; use actix_web::{web, FromRequest, Responder}; -use futures::StreamExt; use gdal::vector::LayerAccess; -use geoengine_datatypes::util::Identifier; use geoengine_operators::util::gdal::gdal_open_dataset; use serde::{Deserialize, Serialize}; -use snafu::ResultExt; use std::path::Path; -use tokio::{fs, io::AsyncWriteExt}; +use tokio::fs; use utoipa::{ToResponse, ToSchema}; pub(crate) fn init_upload_routes(cfg: &mut web::ServiceConfig) @@ -70,42 +67,9 @@ impl<'a> ToSchema<'a> for FileUploadRequest { async fn upload_handler( session: C::Session, app_ctx: web::Data, - mut body: Multipart, + body: Multipart, ) -> Result>> { - let upload_id = UploadId::new(); - - let root = upload_id.root_path()?; - - fs::create_dir_all(&root).await.context(error::Io)?; - - let mut files: Vec = vec![]; - while let Some(item) = body.next().await { - let mut field = item?; - let file_name = field - .content_disposition() - .get_filename() - .ok_or(error::Error::UploadFieldMissingFileName)? - .to_owned(); - - let file_id = FileId::new(); - let mut file = fs::File::create(root.join(&file_name)) - .await - .context(error::Io)?; - - let mut byte_size = 0_u64; - while let Some(chunk) = field.next().await { - let bytes = chunk?; - file.write_all(&bytes).await.context(error::Io)?; - byte_size += bytes.len() as u64; - } - file.flush().await.context(error::Io)?; - - files.push(FileUpload { - id: file_id, - name: file_name, - byte_size, - }); - } + let (upload_id, files) = create_upload(body).await?; app_ctx .session_context(session) diff --git a/services/src/contexts/migrations/migration_0010_delete_uploaded_datasets.rs b/services/src/contexts/migrations/migration_0010_delete_uploaded_datasets.rs new file mode 100644 index 000000000..65fde3597 --- /dev/null +++ b/services/src/contexts/migrations/migration_0010_delete_uploaded_datasets.rs @@ -0,0 +1,24 @@ +use async_trait::async_trait; +use tokio_postgres::Transaction; + +use crate::error::Result; + +use super::database_migration::{DatabaseVersion, Migration}; + +/// This migration adds new delete options for uploaded user datasets +pub struct Migration0010DeleteUploadedDatasets; + +#[async_trait] +impl Migration for Migration0010DeleteUploadedDatasets { + fn prev_version(&self) -> Option { + Some("0009_oidc_tokens".into()) + } + + fn version(&self) -> DatabaseVersion { + "0010_delete_uploaded_datasets".into() + } + + async fn migrate(&self, _tx: &Transaction<'_>) -> Result<()> { + Ok(()) + } +} diff --git a/services/src/contexts/migrations/mod.rs b/services/src/contexts/migrations/mod.rs index 61eed30c9..4e43c44f1 100644 --- a/services/src/contexts/migrations/mod.rs +++ b/services/src/contexts/migrations/mod.rs @@ -9,6 +9,7 @@ pub use crate::contexts::migrations::{ migration_0007_owner_role::Migration0007OwnerRole, migration_0008_band_names::Migration0008BandNames, migration_0009_oidc_tokens::Migration0009OidcTokens, + migration_0010_delete_uploaded_datasets::Migration0010DeleteUploadedDatasets, }; pub use database_migration::{ initialize_database, migrate_database, DatabaseVersion, Migration, MigrationResult, @@ -26,6 +27,7 @@ mod migration_0006_ebv_provider; pub mod migration_0007_owner_role; pub mod migration_0008_band_names; pub mod migration_0009_oidc_tokens; +pub mod migration_0010_delete_uploaded_datasets; #[cfg(test)] mod schema_info; @@ -49,6 +51,7 @@ pub fn all_migrations() -> Vec> { Box::new(Migration0007OwnerRole), Box::new(Migration0008BandNames), Box::new(Migration0009OidcTokens), + Box::new(Migration0010DeleteUploadedDatasets), ] } diff --git a/services/src/contexts/mod.rs b/services/src/contexts/mod.rs index 4bbbbe39f..1cb173e81 100644 --- a/services/src/contexts/mod.rs +++ b/services/src/contexts/mod.rs @@ -29,7 +29,7 @@ pub use migrations::{ Migration0002DatasetListingProvider, Migration0003GbifConfig, Migration0004DatasetListingProviderPrio, Migration0005GbifColumnSelection, Migration0006EbvProvider, Migration0007OwnerRole, Migration0008BandNames, - Migration0009OidcTokens, MigrationResult, + Migration0009OidcTokens, Migration0010DeleteUploadedDatasets, MigrationResult, }; pub use postgres::{PostgresContext, PostgresDb, PostgresSessionContext}; pub use session::{MockableSession, Session, SessionId, SimpleSession}; diff --git a/services/src/datasets/upload.rs b/services/src/datasets/upload.rs index 31514ba8f..1c3932e0a 100644 --- a/services/src/datasets/upload.rs +++ b/services/src/datasets/upload.rs @@ -1,3 +1,4 @@ +use actix_multipart::Multipart; use std::fmt::{Display, Formatter}; use std::path::{Path, PathBuf}; @@ -9,7 +10,12 @@ use crate::{ util::config::{self, get_config_element}, }; use async_trait::async_trait; +use futures_util::StreamExt; +use geoengine_datatypes::util::Identifier; use serde::{Deserialize, Deserializer, Serialize}; +use snafu::ResultExt; +use tokio::fs; +use tokio::io::AsyncWriteExt; use utoipa::ToSchema; identifier!(UploadId); @@ -119,6 +125,52 @@ pub struct UploadListing { pub num_files: usize, } +pub async fn create_upload(mut body: Multipart) -> Result<(UploadId, Vec)> { + let upload_id = UploadId::new(); + + let root = upload_id.root_path()?; + + fs::create_dir_all(&root).await.context(error::Io)?; + + let mut files: Vec = vec![]; + while let Some(item) = body.next().await { + let mut field = item?; + let file_name = field + .content_disposition() + .get_filename() + .ok_or(error::Error::UploadFieldMissingFileName)? + .to_owned(); + + let file_id = FileId::new(); + let mut file = fs::File::create(root.join(&file_name)) + .await + .context(error::Io)?; + + let mut byte_size = 0_u64; + while let Some(chunk) = field.next().await { + let bytes = chunk?; + file.write_all(&bytes).await.context(error::Io)?; + byte_size += bytes.len() as u64; + } + file.flush().await.context(error::Io)?; + + files.push(FileUpload { + id: file_id, + name: file_name, + byte_size, + }); + } + + Ok((upload_id, files)) +} + +pub async fn delete_upload(upload_id: UploadId) -> Result<()> { + let root = upload_id.root_path()?; + log::debug!("Deleting {upload_id}"); + fs::remove_dir_all(&root).await.context(error::Io)?; + Ok(()) +} + #[async_trait] pub trait UploadDb { async fn load_upload(&self, upload: UploadId) -> Result; diff --git a/services/src/error.rs b/services/src/error.rs index a0cf2af77..ff93cb2fa 100644 --- a/services/src/error.rs +++ b/services/src/error.rs @@ -491,6 +491,14 @@ pub enum Error { resource_type: String, resource_id: String, }, + + ExpirationTimestampInPast, + IllegalExpirationUpdate { + reason: String, + }, + IllegalDatasetStatus { + status: String, + }, } impl actix_web::error::ResponseError for Error { diff --git a/services/src/pro/api/apidoc.rs b/services/src/pro/api/apidoc.rs index 2801bb4d9..26b256c81 100644 --- a/services/src/pro/api/apidoc.rs +++ b/services/src/pro/api/apidoc.rs @@ -54,6 +54,8 @@ use crate::layers::listing::{ }; use crate::pro; use crate::pro::api::handlers::users::{Quota, UpdateQuota}; +use crate::pro::datasets::Expiration; +use crate::pro::datasets::ExpirationChange; use crate::pro::permissions::{ Permission, PermissionListing, ResourceId, Role, RoleDescription, RoleId, }; @@ -157,7 +159,9 @@ use utoipa::{Modify, OpenApi}; handlers::upload::upload_handler, pro::api::handlers::permissions::add_permission_handler, pro::api::handlers::permissions::remove_permission_handler, - pro::api::handlers::permissions::get_resource_permissions_handler + pro::api::handlers::permissions::get_resource_permissions_handler, + pro::api::handlers::datasets::set_dataset_expiration, + pro::api::handlers::datasets::gc_expired_datasets ), components( responses( @@ -370,6 +374,8 @@ use utoipa::{Modify, OpenApi}; Volume, VolumeName, DataPath, + Expiration, + ExpirationChange, PlotOutputFormat, WrappedPlotOutput, diff --git a/services/src/pro/api/handlers/datasets.rs b/services/src/pro/api/handlers/datasets.rs index 1eaf7a728..36df42763 100644 --- a/services/src/pro/api/handlers/datasets.rs +++ b/services/src/pro/api/handlers/datasets.rs @@ -1,11 +1,15 @@ +use crate::api::handlers::datasets::add_tag; +use crate::datasets::listing::DatasetProvider; +use crate::datasets::upload::{UploadDb, UploadId}; +use crate::datasets::DatasetName; +use crate::pro::datasets::{ChangeDatasetExpiration, ExpirationChange, UploadedUserDatasetStore}; use crate::{ api::{ handlers::datasets::{ - adjust_meta_data_path, auto_create_dataset_handler, create_upload_dataset, - delete_dataset_handler, get_dataset_handler, get_loading_info_handler, - list_datasets_handler, list_volumes_handler, suggest_meta_data_handler, - update_dataset_handler, update_dataset_provenance_handler, - update_dataset_symbology_handler, + adjust_meta_data_path, auto_create_dataset_handler, delete_dataset_handler, + get_dataset_handler, get_loading_info_handler, list_datasets_handler, + list_volumes_handler, suggest_meta_data_handler, update_dataset_handler, + update_dataset_provenance_handler, update_dataset_symbology_handler, }, model::{ responses::datasets::{errors::*, DatasetNameResponse}, @@ -17,6 +21,7 @@ use crate::{ storage::DatasetStore, upload::{Volume, VolumeName}, }, + error, error::Result, pro::{ contexts::{ProApplicationContext, ProGeoEngineDb}, @@ -24,7 +29,7 @@ use crate::{ }, util::config::{get_config_element, Data}, }; -use actix_web::{web, FromRequest}; +use actix_web::{web, FromRequest, HttpResponse, HttpResponseBuilder, Responder}; use geoengine_datatypes::error::BoxedResultExt; use snafu::ResultExt; @@ -39,6 +44,7 @@ where .service(web::resource("/suggest").route(web::get().to(suggest_meta_data_handler::))) .service(web::resource("/auto").route(web::post().to(auto_create_dataset_handler::))) .service(web::resource("/volumes").route(web::get().to(list_volumes_handler::))) + .service(web::resource("/gc").route(web::post().to(gc_expired_datasets::))) .service( web::resource("/{dataset}/loadingInfo") .route(web::get().to(get_loading_info_handler::)), @@ -51,6 +57,10 @@ where web::resource("/{dataset}/provenance") .route(web::put().to(update_dataset_provenance_handler::)), ) + .service( + web::resource("/{dataset}/expiration") + .route(web::put().to(set_dataset_expiration::)), + ) .service( web::resource("/{dataset}") .route(web::get().to(get_dataset_handler::)) @@ -88,13 +98,13 @@ where let create = create.into_inner(); match create { CreateDataset { - data_path: DataPath::Volume(upload), + data_path: DataPath::Volume(volume), definition, - } => create_system_dataset(session, app_ctx, upload, definition).await, + } => create_system_dataset(session, app_ctx, volume, definition).await, CreateDataset { - data_path: DataPath::Upload(volume), + data_path: DataPath::Upload(upload), definition, - } => create_upload_dataset(session, app_ctx, volume, definition).await, + } => create_upload_dataset(session, app_ctx, upload, definition).await, } } @@ -145,6 +155,135 @@ where Ok(web::Json(dataset.name.into())) } +pub async fn create_upload_dataset( + session: C::Session, + app_ctx: web::Data, + upload_id: UploadId, + mut definition: DatasetDefinition, +) -> Result, CreateDatasetError> +where + <::SessionContext as SessionContext>::GeoEngineDB: ProGeoEngineDb, +{ + let db = app_ctx.session_context(session).db(); + let upload = db.load_upload(upload_id).await.context(UploadNotFound)?; + + add_tag(&mut definition.properties, "upload".to_owned()); + + adjust_meta_data_path(&mut definition.meta_data, &upload) + .context(CannotResolveUploadFilePath)?; + + let result = db + .add_uploaded_dataset( + upload_id, + definition.properties.into(), + definition.meta_data.into(), + ) + .await + .context(CannotCreateDataset)?; + + Ok(web::Json(result.name.into())) +} + +/// Sets an expiration date for the dataset with the given name. +/// Will expire immediately if no timestamp is provided. +#[utoipa::path( + tag = "Datasets", + put, + path = "/dataset/{dataset}/expiration", + request_body(content = ExpirationChange, examples( + ("Delete" = (value = json!({ + "type": "setExpire", + "delete_record": true, + "delete_data": true + }))), + ("Expire" = (value = json!({ + "type": "setExpire", + "delete_record": true, + "delete_data": true, + "deletion_timestamp": "2024-06-28T14:52:39.655Z" + }))), + ("Undo Expire" = (value = json!({ + "type": "unsetExpire" + }))) + )), + responses( + (status = 200, description = "OK"), + (status = 400, description = "Bad request", body = ErrorResponse), + (status = 401, response = crate::api::model::responses::UnauthorizedUserResponse) + ), + params( + ("dataset" = DatasetName, description = "Dataset Name"), + ), + security( + ("session_token" = []) + ) +)] +async fn set_dataset_expiration( + session: C::Session, + app_ctx: web::Data, + dataset: web::Path, + expiration: web::Json, +) -> Result +where + <::SessionContext as SessionContext>::GeoEngineDB: ProGeoEngineDb, +{ + let db = app_ctx.session_context(session).db(); + + let dataset_name = dataset.into_inner(); + + let dataset_id = db.resolve_dataset_name_to_id(&dataset_name).await?; + + let dataset_id = dataset_id.ok_or(error::Error::UnknownDatasetName { + dataset_name: dataset_name.to_string(), + })?; + + let expire_dataset = ChangeDatasetExpiration { + dataset_id, + expiration_change: expiration.into_inner(), + }; + + db.expire_uploaded_dataset(expire_dataset).await?; + + Ok(HttpResponse::Ok()) +} + +/// Clears expired datasets. +/// Requires an admin session. +#[utoipa::path( + tag = "Datasets", + post, + path = "/dataset/gc", + responses( + (status = 200, description = "OK"), + (status = 400, description = "Bad request", body = ErrorResponse), + (status = 401, response = crate::api::model::responses::UnauthorizedUserResponse) + ), + security( + ("session_token" = []) + ) +)] +async fn gc_expired_datasets( + session: C::Session, + app_ctx: web::Data, +) -> Result +where + <::SessionContext as SessionContext>::GeoEngineDB: ProGeoEngineDb, +{ + if !session.is_admin() { + return Err(error::Error::Unauthorized { + source: Box::new(error::Error::OperationRequiresAdminPrivilige), + }); + } + + app_ctx + .session_context(session) + .db() + .clear_expired_datasets() + .await?; + + Ok(HttpResponse::Ok()) +} + #[cfg(test)] mod tests { use super::*; @@ -152,6 +291,7 @@ mod tests { use crate::datasets::DatasetName; use crate::pro::contexts::ProPostgresContext; use crate::pro::ge_context; + use crate::util::tests::get_db_timestamp; use crate::{ api::model::services::{AddDataset, DataPath, DatasetDefinition, MetaDataDefinition}, contexts::{Session, SessionContext, SessionId}, @@ -169,7 +309,7 @@ mod tests { use actix_web_httpauth::headers::authorization::Bearer; use futures::TryStreamExt; use geoengine_datatypes::dataset::NamedData; - use geoengine_datatypes::primitives::ColumnSelection; + use geoengine_datatypes::primitives::{ColumnSelection, Duration}; use geoengine_datatypes::{ collections::{GeometryCollection, MultiPointCollection}, primitives::{BoundingBox2D, SpatialResolution, VectorQueryRectangle}, @@ -185,6 +325,7 @@ mod tests { util::gdal::create_ndvi_meta_data, }; use serde_json::json; + use std::ops::Add; use tokio_postgres::NoTls; pub async fn upload_ne_10m_ports_files( @@ -542,4 +683,53 @@ mod tests { Ok(()) } + + #[ge_context::test] + async fn it_expires_dataset(app_ctx: ProPostgresContext) -> Result<()> { + let mut test_data = TestDataUploads::default(); + + let session = app_ctx.create_anonymous_session().await.unwrap(); + let session_id = session.id(); + let ctx = app_ctx.session_context(session); + + let upload_id = upload_ne_10m_ports_files(app_ctx.clone(), session_id).await?; + test_data.uploads.push(upload_id); + + let dataset_name = + construct_dataset_from_upload(app_ctx.clone(), upload_id, session_id).await; + + let db = ctx.db(); + let dataset_id = db + .resolve_dataset_name_to_id(&dataset_name) + .await + .unwrap() + .unwrap(); + + assert!(db.load_dataset(&dataset_id).await.is_ok()); + + let current_time = get_db_timestamp(&app_ctx).await; + let future_time = current_time.add(Duration::seconds(2)); + + let expiration = + ChangeDatasetExpiration::expire_full(dataset_id, future_time).expiration_change; + + let req = actix_web::test::TestRequest::put() + .uri(&format!("/dataset/{dataset_name}/expiration")) + .set_json(expiration) + .append_header((header::CONTENT_LENGTH, 0)) + .append_header((header::AUTHORIZATION, Bearer::new(session_id.to_string()))) + .append_header((header::CONTENT_TYPE, "application/json")); + + let res = send_pro_test_request(req, app_ctx.clone()).await; + + assert_eq!(res.status(), 200, "response: {res:?}"); + + assert!(db.load_dataset(&dataset_id).await.is_ok()); + + tokio::time::sleep(std::time::Duration::from_secs(2)).await; + + assert!(db.load_dataset(&dataset_id).await.is_err()); + + Ok(()) + } } diff --git a/services/src/pro/contexts/migrations/current_schema.sql b/services/src/pro/contexts/migrations/current_schema.sql index 1dd5244a4..77baf797f 100644 --- a/services/src/pro/contexts/migrations/current_schema.sql +++ b/services/src/pro/contexts/migrations/current_schema.sql @@ -227,3 +227,25 @@ CREATE TABLE oidc_session_tokens ( refresh_token bytea, refresh_token_encryption_nonce bytea ); + +CREATE TYPE "InternalUploadedDatasetStatus" AS ENUM ( + 'Available', + 'Expires', + 'Expired', + 'UpdateExpired', + 'Deleted', + 'DeletedWithError' +); + +CREATE TABLE uploaded_user_datasets ( + user_id uuid, + upload_id uuid, + dataset_id uuid, + status "InternalUploadedDatasetStatus" NOT NULL, + created timestamp with time zone NOT NULL, + expiration timestamp with time zone, + deleted timestamp with time zone, + delete_data boolean NOT NULL, + delete_record boolean NOT NULL, + PRIMARY KEY (user_id, dataset_id, upload_id) +); diff --git a/services/src/pro/contexts/migrations/migration_0010_delete_uploaded_datasets.rs b/services/src/pro/contexts/migrations/migration_0010_delete_uploaded_datasets.rs new file mode 100644 index 000000000..5792e9eaa --- /dev/null +++ b/services/src/pro/contexts/migrations/migration_0010_delete_uploaded_datasets.rs @@ -0,0 +1,39 @@ +use async_trait::async_trait; +use tokio_postgres::Transaction; + +use super::database_migration::{ProMigration, ProMigrationImpl}; +use crate::{contexts::Migration0010DeleteUploadedDatasets, error::Result}; + +#[async_trait] +impl ProMigration for ProMigrationImpl { + async fn pro_migrate(&self, tx: &Transaction<'_>) -> Result<()> { + tx.batch_execute( + r#" + CREATE TYPE "InternalUploadedDatasetStatus" AS ENUM ( + 'Available', + 'Expires', + 'Expired', + 'UpdateExpired', + 'Deleted', + 'DeletedWithError' + ); + + CREATE TABLE uploaded_user_datasets ( + user_id uuid, + upload_id uuid, + dataset_id uuid, + status "InternalUploadedDatasetStatus" NOT NULL, + created timestamp with time zone NOT NULL, + expiration timestamp with time zone, + deleted timestamp with time zone, + delete_data boolean NOT NULL, + delete_record boolean NOT NULL, + PRIMARY KEY (user_id, dataset_id, upload_id) + ); + "#, + ) + .await?; + + Ok(()) + } +} diff --git a/services/src/pro/contexts/migrations/mod.rs b/services/src/pro/contexts/migrations/mod.rs index e80dd5ebe..618958cdc 100644 --- a/services/src/pro/contexts/migrations/mod.rs +++ b/services/src/pro/contexts/migrations/mod.rs @@ -1,4 +1,5 @@ use crate::contexts::migrations::migration_0009_oidc_tokens::Migration0009OidcTokens; +use crate::contexts::migrations::Migration0010DeleteUploadedDatasets; use crate::contexts::Migration; use crate::contexts::{ Migration0000Initial, Migration0001RasterStacks, Migration0002DatasetListingProvider, @@ -16,6 +17,7 @@ mod migration_0000_initial; mod migration_0004_dataset_listing_provider_prio; mod migration_0007_owner_role; mod migration_0009_oidc_tokens; +mod migration_0010_delete_uploaded_datasets; /// Get all regular and pro migrations. This function wraps all regular migrations into a pro migration. pub fn pro_migrations() -> Vec> @@ -36,6 +38,7 @@ where Box::new(NoProMigrationImpl::from(Migration0007OwnerRole)), Box::new(NoProMigrationImpl::from(Migration0008BandNames)), Box::new(ProMigrationImpl::from(Migration0009OidcTokens)), + Box::new(ProMigrationImpl::from(Migration0010DeleteUploadedDatasets)), ] } diff --git a/services/src/pro/contexts/mod.rs b/services/src/pro/contexts/mod.rs index fe824252f..9e3909507 100644 --- a/services/src/pro/contexts/mod.rs +++ b/services/src/pro/contexts/mod.rs @@ -37,6 +37,7 @@ use super::users::{RoleDb, UserAuth, UserSession}; use super::util::config::{Cache, QuotaTrackingMode}; use crate::util::config::get_config_element; +use crate::pro::datasets::UploadedUserDatasetStore; pub use postgres::ProPostgresDb; /// A pro application contexts that extends the default context. @@ -44,7 +45,10 @@ pub trait ProApplicationContext: ApplicationContext + Use fn oidc_manager(&self) -> &OidcManager; } -pub trait ProGeoEngineDb: GeoEngineDb + UserDb + PermissionDb + RoleDb + MlModelDb {} +pub trait ProGeoEngineDb: + GeoEngineDb + UserDb + PermissionDb + RoleDb + MlModelDb + UploadedUserDatasetStore +{ +} pub struct ExecutionContextImpl where diff --git a/services/src/pro/datasets/mod.rs b/services/src/pro/datasets/mod.rs index fe9dfc9a5..c96e11de3 100644 --- a/services/src/pro/datasets/mod.rs +++ b/services/src/pro/datasets/mod.rs @@ -1,7 +1,12 @@ mod external; mod postgres; +mod storage; pub use external::{ GdalRetries, SentinelS2L2ACogsProviderDefinition, StacApiRetries, StacBand, StacZone, TypedProDataProviderDefinition, }; + +pub use storage::{ + ChangeDatasetExpiration, Expiration, ExpirationChange, UploadedUserDatasetStore, +}; diff --git a/services/src/pro/datasets/postgres.rs b/services/src/pro/datasets/postgres.rs index 5819893cb..177a61ba2 100644 --- a/services/src/pro/datasets/postgres.rs +++ b/services/src/pro/datasets/postgres.rs @@ -4,11 +4,19 @@ use crate::datasets::listing::{DatasetListOptions, DatasetListing, DatasetProvid use crate::datasets::listing::{OrderBy, ProvenanceOutput}; use crate::datasets::postgres::resolve_dataset_name_to_id; use crate::datasets::storage::{Dataset, DatasetDb, DatasetStore, MetaDataDefinition}; -use crate::datasets::upload::FileId; +use crate::datasets::upload::{delete_upload, FileId}; use crate::datasets::upload::{Upload, UploadDb, UploadId}; use crate::datasets::{AddDataset, DatasetIdAndName, DatasetName}; +use crate::error::Error::{ + ExpirationTimestampInPast, IllegalDatasetStatus, IllegalExpirationUpdate, UnknownDatasetId, +}; use crate::error::{self, Error, Result}; use crate::pro::contexts::ProPostgresDb; +use crate::pro::datasets::storage::{ + ChangeDatasetExpiration, InternalUploadedDatasetStatus, UploadedDatasetStatus, + UploadedUserDatasetStore, +}; +use crate::pro::datasets::{Expiration, ExpirationChange}; use crate::pro::permissions::postgres_permissiondb::TxPermissionDb; use crate::pro::permissions::{Permission, RoleId}; use crate::projects::Symbology; @@ -28,6 +36,8 @@ use geoengine_operators::engine::{ use geoengine_operators::mock::MockDatasetDataSourceLoadingInfo; use geoengine_operators::source::{GdalLoadingInfo, OgrSourceDataset}; use postgres_types::{FromSql, ToSql}; +use snafu::ensure; +use tokio_postgres::Transaction; impl DatasetDb for ProPostgresDb where @@ -48,6 +58,7 @@ where <>::TlsConnect as TlsConnect>::Future: Send, { async fn list_datasets(&self, options: DatasetListOptions) -> Result> { + self.update_datasets_status().await?; let conn = self.conn_pool.get().await?; let mut pos = 3; @@ -68,7 +79,7 @@ where pos += 1; (format!("AND d.tags @> ${pos}::text[]"), filter_tags.clone()) } else { - (String::new(), vec![]) + ("AND NOT d.tags @> '{deleted}'::text[]".to_string(), vec![]) }; let stmt = conn @@ -167,6 +178,7 @@ where } async fn load_dataset(&self, dataset: &DatasetId) -> Result { + self.update_dataset_status(dataset).await?; let conn = self.conn_pool.get().await?; let stmt = conn .prepare( @@ -211,6 +223,7 @@ where } async fn load_provenance(&self, dataset: &DatasetId) -> Result { + self.update_dataset_status(dataset).await?; let conn = self.conn_pool.get().await?; let stmt = conn @@ -239,6 +252,7 @@ where } async fn load_loading_info(&self, dataset: &DatasetId) -> Result { + self.update_dataset_status(dataset).await?; let conn = self.conn_pool.get().await?; let stmt = conn @@ -265,6 +279,7 @@ where &self, dataset_name: &DatasetName, ) -> Result> { + self.update_datasets_status().await?; let conn = self.conn_pool.get().await?; resolve_dataset_name_to_id(&conn, dataset_name).await } @@ -276,6 +291,7 @@ where limit: u32, offset: u32, ) -> Result> { + self.update_datasets_status().await?; let connection = self.conn_pool.get().await?; let limit = i64::from(limit); @@ -384,6 +400,15 @@ where return Err(geoengine_operators::error::Error::PermissionDenied); }; + let uploaded_status = self.uploaded_dataset_status_in_tx(&id, &tx).await; + if let Ok(status) = uploaded_status { + if matches!(status, UploadedDatasetStatus::Deleted) { + return Err(geoengine_operators::error::Error::DatasetDeleted { + id: id.to_string(), + }); + } + } + let stmt = tx .prepare( " @@ -468,6 +493,15 @@ where return Err(geoengine_operators::error::Error::PermissionDenied); }; + let uploaded_status = self.uploaded_dataset_status_in_tx(&id, &tx).await; + if let Ok(status) = uploaded_status { + if matches!(status, UploadedDatasetStatus::Deleted) { + return Err(geoengine_operators::error::Error::DatasetDeleted { + id: id.to_string(), + }); + } + } + let stmt = tx .prepare( " @@ -721,36 +755,53 @@ where let mut conn = self.conn_pool.get().await?; let tx = conn.build_transaction().start().await?; - self.ensure_permission_in_tx(dataset_id.into(), Permission::Owner, &tx) - .await - .boxed_context(crate::error::PermissionDb)?; - - let stmt = tx - .prepare( - " - SELECT - TRUE - FROM - user_permitted_datasets p JOIN datasets d - ON (p.dataset_id = d.id) - WHERE - d.id = $1 AND p.user_id = $2 AND p.permission = 'Owner';", - ) - .await?; - - let rows = tx - .query(&stmt, &[&dataset_id, &self.session.user.id]) - .await?; - - if rows.is_empty() { - return Err(Error::OperationRequiresOwnerPermission); + let _uploaded = self.uploaded_dataset_status_in_tx(&dataset_id, &tx).await; + if let Err(error) = _uploaded { + if matches!(error, UnknownDatasetId) { + self.ensure_permission_in_tx(dataset_id.into(), Permission::Owner, &tx) + .await + .boxed_context(crate::error::PermissionDb)?; + + let stmt = tx + .prepare( + " + SELECT + TRUE + FROM + user_permitted_datasets p JOIN datasets d + ON (p.dataset_id = d.id) + WHERE + d.id = $1 AND p.user_id = $2 AND p.permission = 'Owner';", + ) + .await?; + + let rows = tx + .query(&stmt, &[&dataset_id, &self.session.user.id]) + .await?; + + if rows.is_empty() { + return Err(Error::OperationRequiresOwnerPermission); + } + + let stmt = tx.prepare("DELETE FROM datasets WHERE id = $1;").await?; + + tx.execute(&stmt, &[&dataset_id]).await?; + + tx.commit().await?; + + return Ok(()); + } } - let stmt = tx.prepare("DELETE FROM datasets WHERE id = $1;").await?; - - tx.execute(&stmt, &[&dataset_id]).await?; - - tx.commit().await?; + let expire_dataset = ChangeDatasetExpiration { + dataset_id, + expiration_change: ExpirationChange::SetExpire(Expiration { + deletion_timestamp: None, + delete_record: true, + delete_data: false, + }), + }; + self.expire_uploaded_dataset(expire_dataset).await?; Ok(()) } @@ -861,11 +912,630 @@ impl From for crate::datasets::upload::FileUpload { } } +#[async_trait] +impl UploadedUserDatasetStore for ProPostgresDb +where + Tls: MakeTlsConnect + Clone + Send + Sync + 'static + std::fmt::Debug, + >::Stream: Send + Sync, + >::TlsConnect: Send, + <>::TlsConnect as TlsConnect>::Future: Send, +{ + async fn add_uploaded_dataset( + &self, + upload_id: UploadId, + dataset: AddDataset, + meta_data: MetaDataDefinition, + ) -> Result { + let id = DatasetId::new(); + let name = dataset.name.unwrap_or_else(|| DatasetName { + namespace: Some(self.session.user.id.to_string()), + name: id.to_string(), + }); + + log::info!( + "Adding dataset with name: {:?}, tags: {:?}", + name, + dataset.tags + ); + + if let Some(tags) = &dataset.tags { + if tags.contains(&"deleted".to_string()) { + log::warn!("Adding a new dataset with a deleted tag"); + } + } + + self.check_namespace(&name)?; + + let typed_meta_data = meta_data.to_typed_metadata(); + + let mut conn = self.conn_pool.get().await?; + + let tx = conn.build_transaction().start().await?; + + tx.execute( + " + INSERT INTO datasets ( + id, + name, + display_name, + description, + source_operator, + result_descriptor, + meta_data, + symbology, + provenance, + tags + ) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10::text[])", + &[ + &id, + &name, + &dataset.display_name, + &dataset.description, + &dataset.source_operator, + &typed_meta_data.result_descriptor, + typed_meta_data.meta_data, + &dataset.symbology, + &dataset.provenance, + &dataset.tags, + ], + ) + .await + .map_unique_violation("datasets", "name", || error::Error::InvalidDatasetName)?; + + let stmt = tx + .prepare( + " + INSERT INTO permissions ( + role_id, + dataset_id, + permission + ) + VALUES ($1, $2, $3)", + ) + .await?; + + tx.execute( + &stmt, + &[&RoleId::from(self.session.user.id), &id, &Permission::Owner], + ) + .await?; + + let stmt = tx + .prepare( + " + INSERT INTO uploaded_user_datasets ( + user_id, + upload_id, + dataset_id, + status, + created, + expiration, + deleted, + delete_data, + delete_record + ) + VALUES ($1, $2, $3, 'Available', CURRENT_TIMESTAMP, NULL, NULL, false, false)", + ) + .await?; + + tx.execute( + &stmt, + &[&RoleId::from(self.session.user.id), &upload_id, &id], + ) + .await?; + + tx.commit().await?; + + Ok(DatasetIdAndName { id, name }) + } + + async fn expire_uploaded_dataset(&self, expire_dataset: ChangeDatasetExpiration) -> Result<()> { + let mut conn = self.conn_pool.get().await?; + let tx = conn.build_transaction().start().await?; + + self.ensure_permission_in_tx(expire_dataset.dataset_id.into(), Permission::Owner, &tx) + .await + .boxed_context(crate::error::PermissionDb)?; + + self.update_dataset_status_in_tx(&tx, &expire_dataset.dataset_id) + .await?; + + match expire_dataset.expiration_change { + ExpirationChange::SetExpire(expiration) => { + let num_changes = if let Some(delete_timestamp) = expiration.deletion_timestamp { + let stmt = tx + .prepare(" + UPDATE uploaded_user_datasets + SET status = 'Expires', expiration = $2, delete_data = $3, delete_record = $4 + WHERE dataset_id = $1 AND $2 >= CURRENT_TIMESTAMP AND (status = 'Available' OR status = 'Expires');", + ).await?; + tx.execute( + &stmt, + &[ + &expire_dataset.dataset_id, + &delete_timestamp, + &expiration.delete_data, + &expiration.delete_record, + ], + ) + .await? + } else { + let stmt = tx.prepare(" + UPDATE uploaded_user_datasets + SET status = 'Expires', expiration = CURRENT_TIMESTAMP, delete_data = $2, delete_record = $3 + WHERE dataset_id = $1 AND (status = 'Available' OR status = 'Expires');", + ).await?; + let num_expired = tx + .execute( + &stmt, + &[ + &expire_dataset.dataset_id, + &expiration.delete_data, + &expiration.delete_record, + ], + ) + .await?; + + if num_expired == 0 { + let stmt = tx + .prepare( + " + UPDATE uploaded_user_datasets + SET delete_data = (CASE WHEN NOT delete_data THEN $2 ELSE true END), + delete_record = (CASE WHEN NOT delete_record THEN $3 ELSE true END), + status = 'UpdateExpired' + WHERE dataset_id = $1 AND (status = 'Expired' OR status = 'Deleted') + AND (delete_data = false OR delete_record = false);", + ) + .await?; + tx.execute( + &stmt, + &[ + &expire_dataset.dataset_id, + &expiration.delete_data, + &expiration.delete_record, + ], + ) + .await? + } else { + num_expired + } + }; + + if num_changes == 0 { + self.validate_expiration_request_in_tx( + &tx, + &expire_dataset.dataset_id, + &expiration, + ) + .await?; + }; + } + ExpirationChange::UnsetExpire => { + let stmt = tx.prepare(" + UPDATE uploaded_user_datasets + SET status = 'Available', expiration = NULL, delete_data = false, delete_record = false + WHERE dataset_id = $1 AND status = 'Expires';", + ).await?; + let set_changes = tx.execute(&stmt, &[&expire_dataset.dataset_id]).await?; + if set_changes == 0 { + return Err(IllegalDatasetStatus { + status: "Requested dataset does not exist or does not have an expiration" + .to_string(), + }); + } + } + } + self.update_dataset_status_in_tx(&tx, &expire_dataset.dataset_id) + .await?; + + tx.commit().await?; + + Ok(()) + } + + async fn validate_expiration_request_in_tx( + &self, + tx: &Transaction, + dataset_id: &DatasetId, + expiration: &Expiration, + ) -> Result<()> { + let (status, delete_data, delete_record, legal_expiration) = + if let Some(timestamp) = expiration.deletion_timestamp { + let stmt = tx + .prepare( + " + SELECT + status, + delete_data, + delete_record, + $2 >= CURRENT_TIMESTAMP as legal_expiration + FROM + uploaded_user_datasets + WHERE + dataset_id = $1;", + ) + .await?; + let row = tx + .query_opt(&stmt, &[&dataset_id, ×tamp]) + .await? + .ok_or(UnknownDatasetId)?; + ( + row.get(0), + row.get(1), + row.get(2), + row.get::(3), + ) + } else { + let stmt = tx + .prepare( + " + SELECT + status, + delete_data, + delete_record, + TRUE as legal_expiration + FROM + uploaded_user_datasets + WHERE + dataset_id = $1;", + ) + .await?; + let row = tx + .query_opt(&stmt, &[&dataset_id]) + .await? + .ok_or(UnknownDatasetId)?; + ( + row.get(0), + row.get(1), + row.get(2), + row.get::(3), + ) + }; + + match status { + InternalUploadedDatasetStatus::Available | InternalUploadedDatasetStatus::Expires => { + if !legal_expiration { + return Err(ExpirationTimestampInPast); + } + } + InternalUploadedDatasetStatus::Expired + | InternalUploadedDatasetStatus::UpdateExpired + | InternalUploadedDatasetStatus::Deleted => { + let data_downgrade = delete_data && !expiration.delete_data; + let record_downgrade = delete_record && !expiration.delete_record; + if data_downgrade || record_downgrade { + let data = if data_downgrade { " data" } else { "" }; + let record = if record_downgrade { " record" } else { "" }; + return Err(IllegalExpirationUpdate { + reason: format!("Prior expiration deleted: {data} {record}"), + }); + } + if expiration.deletion_timestamp.is_some() { + return Err(IllegalExpirationUpdate { + reason: "Setting expiration after deletion".to_string(), + }); + } + } + InternalUploadedDatasetStatus::DeletedWithError => { + return Err(IllegalDatasetStatus { + status: "Dataset was deleted, but an error occurred during deletion" + .to_string(), + }); + } + } + Ok(()) + } + + async fn uploaded_dataset_status_in_tx( + &self, + dataset_id: &DatasetId, + tx: &Transaction, + ) -> Result { + self.ensure_permission_in_tx((*dataset_id).into(), Permission::Read, tx) + .await + .boxed_context(crate::error::PermissionDb)?; + + self.update_dataset_status_in_tx(tx, dataset_id).await?; + + let stmt = tx + .prepare( + " + SELECT + status + FROM + uploaded_user_datasets + WHERE + dataset_id = $1;", + ) + .await?; + + let result = tx + .query_opt(&stmt, &[&dataset_id]) + .await? + .ok_or(error::Error::UnknownDatasetId)?; + + let internal_status: InternalUploadedDatasetStatus = result.get(0); + + Ok(internal_status.into()) + } + + async fn update_dataset_status(&self, dataset_id: &DatasetId) -> Result<()> { + let mut conn = self.conn_pool.get().await?; + let tx = conn.build_transaction().start().await?; + self.update_dataset_status_in_tx(&tx, dataset_id).await?; + tx.commit().await?; + + Ok(()) + } + + async fn update_dataset_status_in_tx( + &self, + tx: &Transaction, + dataset_id: &DatasetId, + ) -> Result<()> { + let mut newly_expired_datasets = 0; + + let delete_records = tx + .prepare( + " + DELETE FROM + datasets + USING + user_permitted_datasets p, uploaded_user_datasets u + WHERE + p.user_id = $1 AND datasets.id = $2 + AND p.dataset_id = datasets.id AND u.dataset_id = datasets.id + AND (u.status = 'Expires' OR u.status = 'UpdateExpired') AND u.expiration <= CURRENT_TIMESTAMP + AND u.delete_record;", + ) + .await?; + newly_expired_datasets += tx + .execute(&delete_records, &[&self.session.user.id, &dataset_id]) + .await?; + + let tag_deletion = tx + .prepare( + " + UPDATE + datasets + SET + tags = tags || '{deleted}' + FROM + user_permitted_datasets p, uploaded_user_datasets u + WHERE + p.user_id = $1 AND datasets.id = $2 + AND p.dataset_id = datasets.id AND u.dataset_id = datasets.id + AND u.status = 'Expires' AND u.expiration <= CURRENT_TIMESTAMP + AND NOT u.delete_record;", + ) + .await?; + newly_expired_datasets += tx + .execute(&tag_deletion, &[&self.session.user.id, &dataset_id]) + .await?; + + if newly_expired_datasets > 0 { + let mark_deletion = tx + .prepare( + " + UPDATE + uploaded_user_datasets + SET + status = 'Expired' + FROM + user_permitted_datasets p + WHERE + p.user_id = $1 AND uploaded_user_datasets.dataset_id = $2 + AND uploaded_user_datasets.dataset_id = p.dataset_id + AND (status = 'Expires' OR status = 'UpdateExpired') AND expiration <= CURRENT_TIMESTAMP;", + ) + .await?; + + tx.execute(&mark_deletion, &[&self.session.user.id, &dataset_id]) + .await?; + } + + Ok(()) + } + + async fn update_datasets_status(&self) -> Result<()> { + let mut conn = self.conn_pool.get().await?; + let tx = conn.build_transaction().start().await?; + self.update_datasets_status_in_tx(&tx).await?; + tx.commit().await?; + + Ok(()) + } + + async fn update_datasets_status_in_tx(&self, tx: &Transaction) -> Result<()> { + if self.session.is_admin() { + let delete_records = tx + .prepare( + " + DELETE FROM + datasets + USING + user_permitted_datasets p, uploaded_user_datasets u + WHERE + u.dataset_id = datasets.id + AND u.status = 'Expires' AND u.expiration <= CURRENT_TIMESTAMP + AND u.delete_record;", + ) + .await?; + tx.execute(&delete_records, &[]).await?; + + let tag_deletion = tx + .prepare( + " + UPDATE + datasets + SET + tags = tags || '{deleted}' + FROM + uploaded_user_datasets u + WHERE + u.dataset_id = datasets.id + AND u.status = 'Expires' AND u.expiration <= CURRENT_TIMESTAMP + AND NOT u.delete_record;", + ) + .await?; + tx.execute(&tag_deletion, &[]).await?; + + let mark_deletion = tx + .prepare( + " + UPDATE + uploaded_user_datasets + SET + status = 'Expired' + WHERE + (status = 'Expires' or status = 'UpdateExpired') AND expiration <= CURRENT_TIMESTAMP;", + ) + .await?; + + tx.execute(&mark_deletion, &[]).await?; + } else { + let delete_records = tx + .prepare( + " + DELETE FROM + datasets + USING + user_permitted_datasets p, uploaded_user_datasets u + WHERE + p.user_id = $1 AND p.dataset_id = datasets.id AND u.dataset_id = datasets.id + AND u.status = 'Expires' AND u.expiration <= CURRENT_TIMESTAMP + AND u.delete_record;", + ) + .await?; + tx.execute(&delete_records, &[&self.session.user.id]) + .await?; + + let tag_deletion = tx + .prepare( + " + UPDATE + datasets + SET + tags = tags || '{deleted}' + FROM + user_permitted_datasets p, uploaded_user_datasets u + WHERE + p.user_id = $1 AND p.dataset_id = datasets.id AND u.dataset_id = datasets.id + AND u.status = 'Expires' AND u.expiration <= CURRENT_TIMESTAMP + AND NOT u.delete_record;", + ) + .await?; + tx.execute(&tag_deletion, &[&self.session.user.id]).await?; + + let mark_deletion = tx + .prepare( + " + UPDATE + uploaded_user_datasets + SET + status = 'Expired' + FROM + user_permitted_datasets p + WHERE + p.user_id = $1 AND uploaded_user_datasets.dataset_id = p.dataset_id + AND (status = 'Expires' OR status = 'UpdateExpired') AND expiration <= CURRENT_TIMESTAMP;", + ) + .await?; + + tx.execute(&mark_deletion, &[&self.session.user.id]).await?; + } + Ok(()) + } + + async fn clear_expired_datasets(&self) -> Result { + ensure!(self.session.is_admin(), error::PermissionDenied); + + let mut conn = self.conn_pool.get().await?; + let tx = conn.build_transaction().start().await?; + + self.update_datasets_status_in_tx(&tx).await?; + + let marked_datasets = tx + .prepare( + " + SELECT + dataset_id, upload_id + FROM + uploaded_user_datasets + WHERE + status = 'Expired' AND delete_data;", + ) + .await?; + + let rows = tx.query(&marked_datasets, &[]).await?; + + let mut deleted = vec![]; + let mut deleted_with_error = vec![]; + + for row in rows { + let dataset_id: DatasetId = row.get(0); + let upload_id = row.get(1); + let res = delete_upload(upload_id).await; + if let Err(error) = res { + log::error!("Error during deletion of upload {upload_id} from dataset {dataset_id}: {error}, marking as DeletedWithError"); + deleted_with_error.push(upload_id); + } else { + deleted.push(upload_id); + } + } + + if !deleted.is_empty() { + let mark_deletion = tx + .prepare( + " + UPDATE + uploaded_user_datasets + SET + status = 'Deleted' + WHERE + status = 'Expired' AND delete_data AND upload_id = ANY($1);", + ) + .await?; + tx.execute(&mark_deletion, &[&deleted]).await?; + } + + if !deleted_with_error.is_empty() { + let mark_error = tx + .prepare( + " + UPDATE + uploaded_user_datasets + SET + status = 'DeletedWithError' + WHERE + status = 'Expired' AND delete_data AND upload_id = ANY($1);", + ) + .await?; + tx.execute(&mark_error, &[&deleted_with_error]).await?; + } + + tx.commit().await?; + + Ok(deleted.len()) + } +} + #[cfg(test)] mod tests { + use std::fs; + use std::ops::{Add, Sub}; use std::path::PathBuf; use super::*; + use crate::api::model::responses::IdResponse; + use crate::contexts::SessionId; + use crate::datasets::upload::UploadRootPath; + use crate::error::Error::PermissionDenied; + use crate::pro::users::{UserCredentials, UserRegistration}; + use crate::pro::util::tests::{admin_login, send_pro_test_request}; + use crate::util::tests::{get_db_timestamp, SetMultipartBody, TestDataUploads}; use crate::{ contexts::{ApplicationContext, SessionContext}, pro::{ @@ -874,11 +1544,16 @@ mod tests { users::{UserAuth, UserSession}, }, }; + use actix_web::http::header; + use actix_web::test; + use actix_web_httpauth::headers::authorization::Bearer; + use geoengine_datatypes::primitives::Duration; use geoengine_datatypes::{ collections::VectorDataType, primitives::{CacheTtlSeconds, FeatureDataType, Measurement}, spatial_reference::SpatialReference, }; + use geoengine_operators::error::Error::DatasetDeleted; use geoengine_operators::{ engine::{StaticMetaData, VectorColumnInfo}, source::{ @@ -1003,4 +1678,608 @@ mod tests { .await .unwrap(); } + + const TEST_POINT_DATASET_SOURCE_PATH: &str = "vector/data/points.fgb"; + + struct TestDatasetDefinition { + meta_data: MetaDataDefinition, + dataset_name: DatasetName, + } + + struct UploadedTestDataset { + dataset_name: DatasetName, + dataset_id: DatasetId, + upload_id: UploadId, + } + + fn test_point_dataset(name_space: String, name: &str) -> TestDatasetDefinition { + let local_path = PathBuf::from(TEST_POINT_DATASET_SOURCE_PATH); + let file_name = local_path.file_name().unwrap().to_str().unwrap(); + let loading_info = OgrSourceDataset { + file_name: PathBuf::from(file_name), + layer_name: file_name.to_owned(), + data_type: Some(VectorDataType::MultiPoint), + time: OgrSourceDatasetTimeType::None, + default_geometry: None, + columns: Some(OgrSourceColumnSpec { + format_specifics: None, + x: "x".to_owned(), + y: Some("y".to_owned()), + int: vec!["num".to_owned()], + float: vec![], + text: vec!["txt".to_owned()], + bool: vec![], + datetime: vec![], + rename: None, + }), + force_ogr_time_filter: false, + force_ogr_spatial_filter: false, + on_error: OgrSourceErrorSpec::Ignore, + sql_query: None, + attribute_query: None, + cache_ttl: CacheTtlSeconds::default(), + }; + + let meta_data = MetaDataDefinition::OgrMetaData(StaticMetaData::< + OgrSourceDataset, + VectorResultDescriptor, + VectorQueryRectangle, + > { + loading_info: loading_info.clone(), + result_descriptor: VectorResultDescriptor { + data_type: VectorDataType::MultiPoint, + spatial_reference: SpatialReference::epsg_4326().into(), + columns: [ + ( + "num".to_owned(), + VectorColumnInfo { + data_type: FeatureDataType::Int, + measurement: Measurement::Unitless, + }, + ), + ( + "txt".to_owned(), + VectorColumnInfo { + data_type: FeatureDataType::Text, + measurement: Measurement::Unitless, + }, + ), + ] + .into_iter() + .collect(), + time: None, + bbox: None, + }, + phantom: Default::default(), + }); + + let dataset_name = DatasetName::new(Some(name_space), name); + + TestDatasetDefinition { + meta_data, + dataset_name, + } + } + + async fn upload_point_dataset( + app_ctx: &ProPostgresContext, + session_id: SessionId, + ) -> UploadId { + let files = + vec![geoengine_datatypes::test_data!(TEST_POINT_DATASET_SOURCE_PATH).to_path_buf()]; + + let req = actix_web::test::TestRequest::post() + .uri("/upload") + .append_header((header::AUTHORIZATION, Bearer::new(session_id.to_string()))) + .set_multipart_files(&files); + + let res = send_pro_test_request(req, app_ctx.clone()).await; + assert_eq!(res.status(), 200); + let upload: IdResponse = test::read_body_json(res).await; + + upload.id + } + + async fn upload_and_add_point_dataset( + app_ctx: &ProPostgresContext, + user_session: &UserSession, + name: &str, + upload_dir: &mut TestDataUploads, + ) -> UploadedTestDataset { + let test_dataset = test_point_dataset(user_session.user.id.to_string(), name); + let upload_id = upload_point_dataset(app_ctx, user_session.id).await; + + let res = app_ctx + .session_context(user_session.clone()) + .db() + .add_uploaded_dataset( + upload_id, + AddDataset { + name: Some(test_dataset.dataset_name.clone()), + display_name: "Ogr Test".to_owned(), + description: "desc".to_owned(), + source_operator: "OgrSource".to_owned(), + symbology: None, + provenance: None, + tags: Some(vec!["upload".to_owned(), "test".to_owned()]), + }, + test_dataset.meta_data.clone(), + ) + .await + .unwrap(); + + upload_dir.uploads.push(upload_id); + + UploadedTestDataset { + dataset_name: test_dataset.dataset_name, + dataset_id: res.id, + upload_id, + } + } + + fn listing_not_deleted(dataset: &DatasetListing, origin: &UploadedTestDataset) -> bool { + dataset.name == origin.dataset_name && !dataset.tags.contains(&"deleted".to_owned()) + } + + fn dataset_deleted(dataset: &Dataset, origin: &UploadedTestDataset) -> bool { + let tags = dataset.tags.clone().unwrap(); + let mut num_deleted = 0; + for tag in tags { + if tag == *"deleted" { + num_deleted += 1; + } + } + dataset.name == origin.dataset_name && num_deleted == 1 + } + + fn dir_exists(origin: &UploadedTestDataset) -> bool { + let path = origin.upload_id.root_path().unwrap(); + fs::read_dir(path).is_ok() + } + + async fn register_test_user(app_ctx: &ProPostgresContext) -> UserSession { + let _user_id = app_ctx + .register_user(UserRegistration { + email: "test@localhost".to_string(), + real_name: "Foo Bar".to_string(), + password: "test".to_string(), + }) + .await + .unwrap(); + + app_ctx + .login(UserCredentials { + email: "test@localhost".to_string(), + password: "test".to_string(), + }) + .await + .unwrap() + } + + #[ge_context::test] + async fn it_deletes_datasets(app_ctx: ProPostgresContext) { + let mut test_data = TestDataUploads::default(); + let user_session = register_test_user(&app_ctx).await; + + let available = + upload_and_add_point_dataset(&app_ctx, &user_session, "available", &mut test_data) + .await; + let fair = + upload_and_add_point_dataset(&app_ctx, &user_session, "fair", &mut test_data).await; + let full = + upload_and_add_point_dataset(&app_ctx, &user_session, "full", &mut test_data).await; + let none = + upload_and_add_point_dataset(&app_ctx, &user_session, "none", &mut test_data).await; + + let db = app_ctx.session_context(user_session.clone()).db(); + + let default_list_options = DatasetListOptions { + filter: None, + order: OrderBy::NameAsc, + offset: 0, + limit: 10, + tags: None, + }; + + let listing = db + .list_datasets(default_list_options.clone()) + .await + .unwrap(); + + assert_eq!(listing.len(), 4); + assert!(listing_not_deleted(listing.first().unwrap(), &available)); + assert!(listing_not_deleted(listing.get(1).unwrap(), &fair)); + assert!(listing_not_deleted(listing.get(2).unwrap(), &full)); + assert!(listing_not_deleted(listing.get(3).unwrap(), &none)); + + db.expire_uploaded_dataset(ChangeDatasetExpiration::delete_fair(fair.dataset_id)) + .await + .unwrap(); + db.expire_uploaded_dataset(ChangeDatasetExpiration::delete_none(none.dataset_id)) + .await + .unwrap(); + db.expire_uploaded_dataset(ChangeDatasetExpiration::delete_full(full.dataset_id)) + .await + .unwrap(); + + let listing = db + .list_datasets(default_list_options.clone()) + .await + .unwrap(); + + assert_eq!(listing.len(), 1); + assert!(listing_not_deleted(listing.first().unwrap(), &available)); + assert!(dataset_deleted( + &db.load_dataset(&fair.dataset_id).await.unwrap(), + &fair + )); + assert!(matches!( + db.load_dataset(&full.dataset_id).await.unwrap_err(), + UnknownDatasetId + )); + assert!(dataset_deleted( + &db.load_dataset(&none.dataset_id).await.unwrap(), + &none + )); + + assert!(dir_exists(&available)); + assert!(dir_exists(&fair)); + assert!(dir_exists(&full)); + assert!(dir_exists(&none)); + + let admin_session = admin_login(&app_ctx).await; + let admin_ctx = app_ctx.session_context(admin_session.clone()); + let deleted = admin_ctx.db().clear_expired_datasets().await.unwrap(); + + assert_eq!(deleted, 2); + assert!(dir_exists(&available)); + assert!(!dir_exists(&fair)); + assert!(!dir_exists(&full)); + assert!(dir_exists(&none)); + + let deleted = admin_ctx.db().clear_expired_datasets().await.unwrap(); + assert_eq!(deleted, 0); + } + + #[ge_context::test] + async fn it_expires_dataset(app_ctx: ProPostgresContext) { + let mut test_data = TestDataUploads::default(); + let user_session = register_test_user(&app_ctx).await; + + let current_time = get_db_timestamp(&app_ctx).await; + let future_time = current_time.add(Duration::seconds(3)); + + let fair = + upload_and_add_point_dataset(&app_ctx, &user_session, "fair", &mut test_data).await; + + let db = app_ctx.session_context(user_session.clone()).db(); + + let default_list_options = DatasetListOptions { + filter: None, + order: OrderBy::NameAsc, + offset: 0, + limit: 10, + tags: None, + }; + + db.expire_uploaded_dataset(ChangeDatasetExpiration::expire_fair( + fair.dataset_id, + future_time, + )) + .await + .unwrap(); + + let listing = db + .list_datasets(default_list_options.clone()) + .await + .unwrap(); + + assert_eq!(listing.len(), 1); + assert!(listing_not_deleted(listing.first().unwrap(), &fair)); + + tokio::time::sleep(std::time::Duration::from_secs(5)).await; + + let listing = db + .list_datasets(default_list_options.clone()) + .await + .unwrap(); + + assert_eq!(listing.len(), 0); + assert!(dataset_deleted( + &db.load_dataset(&fair.dataset_id).await.unwrap(), + &fair + )); + } + + #[ge_context::test] + async fn it_updates_expiring_dataset(app_ctx: ProPostgresContext) { + let mut test_data = TestDataUploads::default(); + let user_session = register_test_user(&app_ctx).await; + + let current_time = get_db_timestamp(&app_ctx).await; + let future_time_1 = current_time.add(Duration::seconds(2)); + let future_time_2 = current_time.add(Duration::seconds(5)); + + let fair = + upload_and_add_point_dataset(&app_ctx, &user_session, "fair", &mut test_data).await; + let fair2full = + upload_and_add_point_dataset(&app_ctx, &user_session, "fair2full", &mut test_data) + .await; + + let db = app_ctx.session_context(user_session.clone()).db(); + + let default_list_options = DatasetListOptions { + filter: None, + order: OrderBy::NameAsc, + offset: 0, + limit: 10, + tags: None, + }; + + db.expire_uploaded_dataset(ChangeDatasetExpiration::expire_fair( + fair.dataset_id, + future_time_1, + )) + .await + .unwrap(); + db.expire_uploaded_dataset(ChangeDatasetExpiration::expire_fair( + fair.dataset_id, + future_time_2, + )) + .await + .unwrap(); + db.expire_uploaded_dataset(ChangeDatasetExpiration::expire_fair( + fair2full.dataset_id, + future_time_1, + )) + .await + .unwrap(); + db.expire_uploaded_dataset(ChangeDatasetExpiration::expire_none( + fair2full.dataset_id, + future_time_1, + )) + .await + .unwrap(); + db.expire_uploaded_dataset(ChangeDatasetExpiration::expire_meta( + fair2full.dataset_id, + future_time_1, + )) + .await + .unwrap(); + db.expire_uploaded_dataset(ChangeDatasetExpiration::expire_fair( + fair2full.dataset_id, + future_time_1, + )) + .await + .unwrap(); + db.expire_uploaded_dataset(ChangeDatasetExpiration::expire_full( + fair2full.dataset_id, + future_time_1, + )) + .await + .unwrap(); + + let listing = db + .list_datasets(default_list_options.clone()) + .await + .unwrap(); + + assert_eq!(listing.len(), 2); + assert!(listing_not_deleted(listing.first().unwrap(), &fair)); + assert!(listing_not_deleted(listing.get(1).unwrap(), &fair2full)); + + tokio::time::sleep(std::time::Duration::from_secs(3)).await; + + let listing = db + .list_datasets(default_list_options.clone()) + .await + .unwrap(); + + assert_eq!(listing.len(), 1); + assert!(listing_not_deleted(listing.first().unwrap(), &fair)); + assert!(matches!( + db.load_dataset(&fair2full.dataset_id).await.unwrap_err(), + UnknownDatasetId + )); + + tokio::time::sleep(std::time::Duration::from_secs(3)).await; + + let listing = db + .list_datasets(default_list_options.clone()) + .await + .unwrap(); + assert_eq!(listing.len(), 0); + assert!(dataset_deleted( + &db.load_dataset(&fair.dataset_id).await.unwrap(), + &fair + )); + } + + #[allow(clippy::too_many_lines)] + #[ge_context::test] + async fn it_updates_expired_dataset(app_ctx: ProPostgresContext) { + let mut test_data = TestDataUploads::default(); + let user_session = register_test_user(&app_ctx).await; + + let db = app_ctx.session_context(user_session.clone()).db(); + let default_list_options = DatasetListOptions { + filter: None, + order: OrderBy::NameAsc, + offset: 0, + limit: 10, + tags: None, + }; + + let fair2full = + upload_and_add_point_dataset(&app_ctx, &user_session, "fair2full", &mut test_data) + .await; + db.expire_uploaded_dataset(ChangeDatasetExpiration::delete_fair(fair2full.dataset_id)) + .await + .unwrap(); + assert!(dataset_deleted( + &db.load_dataset(&fair2full.dataset_id).await.unwrap(), + &fair2full + )); + db.expire_uploaded_dataset(ChangeDatasetExpiration::delete_full(fair2full.dataset_id)) + .await + .unwrap(); + assert!(matches!( + db.load_dataset(&fair2full.dataset_id).await.unwrap_err(), + UnknownDatasetId + )); + + let none2fair = + upload_and_add_point_dataset(&app_ctx, &user_session, "none2fair", &mut test_data) + .await; + db.expire_uploaded_dataset(ChangeDatasetExpiration::delete_none(none2fair.dataset_id)) + .await + .unwrap(); + assert!(dataset_deleted( + &db.load_dataset(&none2fair.dataset_id).await.unwrap(), + &none2fair + )); + db.expire_uploaded_dataset(ChangeDatasetExpiration::delete_fair(none2fair.dataset_id)) + .await + .unwrap(); + assert!(dataset_deleted( + &db.load_dataset(&none2fair.dataset_id).await.unwrap(), + &none2fair + )); + + let none2meta = + upload_and_add_point_dataset(&app_ctx, &user_session, "none2meta", &mut test_data) + .await; + db.expire_uploaded_dataset(ChangeDatasetExpiration::delete_none(none2meta.dataset_id)) + .await + .unwrap(); + assert!(dataset_deleted( + &db.load_dataset(&none2meta.dataset_id).await.unwrap(), + &none2meta + )); + db.expire_uploaded_dataset(ChangeDatasetExpiration::delete_meta(none2meta.dataset_id)) + .await + .unwrap(); + assert!(matches!( + db.load_dataset(&none2meta.dataset_id).await.unwrap_err(), + UnknownDatasetId + )); + + assert!(db + .expire_uploaded_dataset(ChangeDatasetExpiration::delete_none(none2fair.dataset_id)) + .await + .is_err()); + assert!(db + .expire_uploaded_dataset(ChangeDatasetExpiration::delete_none(fair2full.dataset_id)) + .await + .is_err()); + assert!(db + .expire_uploaded_dataset(ChangeDatasetExpiration::delete_fair(fair2full.dataset_id)) + .await + .is_err()); + + let current_time = get_db_timestamp(&app_ctx).await; + let future_time = current_time.add(Duration::seconds(2)); + let fair2available = + upload_and_add_point_dataset(&app_ctx, &user_session, "fair2available", &mut test_data) + .await; + db.expire_uploaded_dataset(ChangeDatasetExpiration::expire_fair( + fair2available.dataset_id, + future_time, + )) + .await + .unwrap(); + db.expire_uploaded_dataset(ChangeDatasetExpiration::unset_expire( + fair2available.dataset_id, + )) + .await + .unwrap(); + + tokio::time::sleep(std::time::Duration::from_secs(5)).await; + + let admin_session = admin_login(&app_ctx).await; + let admin_ctx = app_ctx.session_context(admin_session.clone()); + let deleted = admin_ctx.db().clear_expired_datasets().await.unwrap(); + assert_eq!(deleted, 2); + + let listing = db + .list_datasets(default_list_options.clone()) + .await + .unwrap(); + assert_eq!(listing.len(), 1); + assert!(listing_not_deleted( + listing.first().unwrap(), + &fair2available + )); + + assert!(dir_exists(&fair2available)); + assert!(!dir_exists(&fair2full)); + assert!(!dir_exists(&none2fair)); + assert!(dir_exists(&none2meta)); + } + + #[ge_context::test] + async fn it_handles_expiration_errors(app_ctx: ProPostgresContext) { + let mut test_data = TestDataUploads::default(); + let user_session = register_test_user(&app_ctx).await; + + let current_time = get_db_timestamp(&app_ctx).await; + let future_time = current_time.add(Duration::hours(1)); + let past_time = current_time.sub(Duration::hours(1)); + + let db = app_ctx.session_context(user_session.clone()).db(); + + //Expire before current time + let test_dataset = + upload_and_add_point_dataset(&app_ctx, &user_session, "fair2full", &mut test_data) + .await; + let err = db + .expire_uploaded_dataset(ChangeDatasetExpiration::expire_fair( + test_dataset.dataset_id, + past_time, + )) + .await; + assert!(err.is_err()); + assert!(matches!(err.unwrap_err(), ExpirationTimestampInPast)); + + //Unset expire for not-expiring dataset + let err = db + .expire_uploaded_dataset(ChangeDatasetExpiration::unset_expire( + test_dataset.dataset_id, + )) + .await; + assert!(err.is_err()); + assert!(matches!(err.unwrap_err(), IllegalDatasetStatus { .. })); + + //Expire already deleted + db.expire_uploaded_dataset(ChangeDatasetExpiration::delete_fair( + test_dataset.dataset_id, + )) + .await + .unwrap(); + let err = db + .expire_uploaded_dataset(ChangeDatasetExpiration::expire_fair( + test_dataset.dataset_id, + future_time, + )) + .await; + assert!(err.is_err()); + assert!(matches!(err.unwrap_err(), IllegalExpirationUpdate { .. })); + + // Call meta data for deleted + let err: std::result::Result< + Box>, + geoengine_operators::error::Error, + > = db + .meta_data(&DataId::Internal { + dataset_id: test_dataset.dataset_id, + }) + .await; + assert!(err.is_err()); + assert!(matches!(err.unwrap_err(), DatasetDeleted { .. })); + + //Clear without admin permission + let err = db.clear_expired_datasets().await; + assert!(err.is_err()); + assert!(matches!(err.unwrap_err(), PermissionDenied)); + } } diff --git a/services/src/pro/datasets/storage.rs b/services/src/pro/datasets/storage.rs new file mode 100644 index 000000000..eef64a703 --- /dev/null +++ b/services/src/pro/datasets/storage.rs @@ -0,0 +1,197 @@ +use crate::datasets::storage::MetaDataDefinition; +use crate::datasets::upload::UploadId; +use crate::datasets::{AddDataset, DatasetIdAndName}; +use crate::error::Result; +use async_trait::async_trait; +use geoengine_datatypes::dataset::DatasetId; +use geoengine_datatypes::primitives::DateTime; +use postgres_types::{FromSql, ToSql}; +use serde::{Deserialize, Serialize}; +use tokio_postgres::Transaction; +use utoipa::ToSchema; + +#[derive(Deserialize, Serialize, Debug, Clone, ToSchema)] +pub struct Expiration { + pub deletion_timestamp: Option, + pub delete_record: bool, + pub delete_data: bool, +} + +#[derive(Deserialize, Serialize, Debug, Clone)] +pub struct ChangeDatasetExpiration { + pub dataset_id: DatasetId, + pub expiration_change: ExpirationChange, +} + +impl ChangeDatasetExpiration { + pub fn delete_meta(dataset_id: DatasetId) -> Self { + ChangeDatasetExpiration { + dataset_id, + expiration_change: ExpirationChange::SetExpire(Expiration { + deletion_timestamp: None, + delete_record: true, + delete_data: false, + }), + } + } + + pub fn delete_fair(dataset_id: DatasetId) -> Self { + ChangeDatasetExpiration { + dataset_id, + expiration_change: ExpirationChange::SetExpire(Expiration { + deletion_timestamp: None, + delete_record: false, + delete_data: true, + }), + } + } + + pub fn delete_full(dataset_id: DatasetId) -> Self { + ChangeDatasetExpiration { + dataset_id, + expiration_change: ExpirationChange::SetExpire(Expiration { + deletion_timestamp: None, + delete_record: true, + delete_data: true, + }), + } + } + + pub fn delete_none(dataset_id: DatasetId) -> Self { + ChangeDatasetExpiration { + dataset_id, + expiration_change: ExpirationChange::SetExpire(Expiration { + deletion_timestamp: None, + delete_record: false, + delete_data: false, + }), + } + } + + pub fn expire_meta(dataset_id: DatasetId, timestamp: DateTime) -> Self { + ChangeDatasetExpiration { + dataset_id, + expiration_change: ExpirationChange::SetExpire(Expiration { + deletion_timestamp: Some(timestamp), + delete_record: true, + delete_data: false, + }), + } + } + + pub fn expire_fair(dataset_id: DatasetId, timestamp: DateTime) -> Self { + ChangeDatasetExpiration { + dataset_id, + expiration_change: ExpirationChange::SetExpire(Expiration { + deletion_timestamp: Some(timestamp), + delete_record: false, + delete_data: true, + }), + } + } + + pub fn expire_full(dataset_id: DatasetId, timestamp: DateTime) -> Self { + ChangeDatasetExpiration { + dataset_id, + expiration_change: ExpirationChange::SetExpire(Expiration { + deletion_timestamp: Some(timestamp), + delete_record: true, + delete_data: true, + }), + } + } + + pub fn expire_none(dataset_id: DatasetId, timestamp: DateTime) -> Self { + ChangeDatasetExpiration { + dataset_id, + expiration_change: ExpirationChange::SetExpire(Expiration { + deletion_timestamp: Some(timestamp), + delete_record: false, + delete_data: false, + }), + } + } + + pub fn unset_expire(dataset_id: DatasetId) -> Self { + ChangeDatasetExpiration { + dataset_id, + expiration_change: ExpirationChange::UnsetExpire, + } + } +} + +#[derive(Deserialize, Serialize, Debug, Clone, ToSchema)] +#[serde(rename_all = "camelCase", tag = "type")] +pub enum ExpirationChange { + SetExpire(Expiration), + UnsetExpire, +} + +pub enum UploadedDatasetStatus { + Available, + Deleted, +} + +#[derive(Debug, FromSql, ToSql)] +pub enum InternalUploadedDatasetStatus { + Available, + Expires, + Expired, + UpdateExpired, + Deleted, + DeletedWithError, +} + +impl From for UploadedDatasetStatus { + fn from(value: InternalUploadedDatasetStatus) -> Self { + match value { + InternalUploadedDatasetStatus::Available | InternalUploadedDatasetStatus::Expires => { + UploadedDatasetStatus::Available + } + InternalUploadedDatasetStatus::Expired + | InternalUploadedDatasetStatus::UpdateExpired + | InternalUploadedDatasetStatus::Deleted + | InternalUploadedDatasetStatus::DeletedWithError => UploadedDatasetStatus::Deleted, + } + } +} + +/// Storage of user-uploaded datasets +#[async_trait] +pub trait UploadedUserDatasetStore { + async fn add_uploaded_dataset( + &self, + upload_id: UploadId, + dataset: AddDataset, + meta_data: MetaDataDefinition, + ) -> Result; + + async fn expire_uploaded_dataset(&self, expire_dataset: ChangeDatasetExpiration) -> Result<()>; + + async fn validate_expiration_request_in_tx( + &self, + tx: &Transaction, + dataset_id: &DatasetId, + expiration: &Expiration, + ) -> Result<()>; + + async fn uploaded_dataset_status_in_tx( + &self, + dataset_id: &DatasetId, + tx: &Transaction, + ) -> Result; + + async fn update_dataset_status(&self, dataset_id: &DatasetId) -> Result<()>; + + async fn update_dataset_status_in_tx( + &self, + tx: &Transaction, + dataset_id: &DatasetId, + ) -> Result<()>; + + async fn update_datasets_status(&self) -> Result<()>; + + async fn update_datasets_status_in_tx(&self, tx: &Transaction) -> Result<()>; + + async fn clear_expired_datasets(&self) -> Result; +} diff --git a/services/src/util/tests.rs b/services/src/util/tests.rs index 85383c9eb..c72b033c3 100644 --- a/services/src/util/tests.rs +++ b/services/src/util/tests.rs @@ -589,3 +589,13 @@ where Err(err) => std::panic::resume_unwind(err), } } + +#[cfg(test)] +#[allow(clippy::missing_panics_doc)] +pub async fn get_db_timestamp( + app_ctx: &crate::pro::contexts::ProPostgresContext, +) -> geoengine_datatypes::primitives::DateTime { + let conn = app_ctx.pool.get().await.unwrap(); + let get_time_stmt = conn.prepare("SELECT CURRENT_TIMESTAMP;").await.unwrap(); + conn.query_one(&get_time_stmt, &[]).await.unwrap().get(0) +} From 40c13030535e8caf756d2141d98fe716b13a9697 Mon Sep 17 00:00:00 2001 From: Nikolaus Glombiewski Date: Wed, 3 Jul 2024 15:00:37 +0200 Subject: [PATCH 02/17] Move db timestamp query to pro utils; --- services/src/pro/api/handlers/datasets.rs | 2 +- services/src/pro/datasets/postgres.rs | 3 ++- services/src/pro/util/tests.rs | 8 ++++++++ services/src/util/tests.rs | 10 ---------- 4 files changed, 11 insertions(+), 12 deletions(-) diff --git a/services/src/pro/api/handlers/datasets.rs b/services/src/pro/api/handlers/datasets.rs index 36df42763..b3e41fc1f 100644 --- a/services/src/pro/api/handlers/datasets.rs +++ b/services/src/pro/api/handlers/datasets.rs @@ -291,7 +291,7 @@ mod tests { use crate::datasets::DatasetName; use crate::pro::contexts::ProPostgresContext; use crate::pro::ge_context; - use crate::util::tests::get_db_timestamp; + use crate::pro::util::tests::get_db_timestamp; use crate::{ api::model::services::{AddDataset, DataPath, DatasetDefinition, MetaDataDefinition}, contexts::{Session, SessionContext, SessionId}, diff --git a/services/src/pro/datasets/postgres.rs b/services/src/pro/datasets/postgres.rs index 177a61ba2..e2f9bf569 100644 --- a/services/src/pro/datasets/postgres.rs +++ b/services/src/pro/datasets/postgres.rs @@ -1534,8 +1534,9 @@ mod tests { use crate::datasets::upload::UploadRootPath; use crate::error::Error::PermissionDenied; use crate::pro::users::{UserCredentials, UserRegistration}; + use crate::pro::util::tests::get_db_timestamp; use crate::pro::util::tests::{admin_login, send_pro_test_request}; - use crate::util::tests::{get_db_timestamp, SetMultipartBody, TestDataUploads}; + use crate::util::tests::{SetMultipartBody, TestDataUploads}; use crate::{ contexts::{ApplicationContext, SessionContext}, pro::{ diff --git a/services/src/pro/util/tests.rs b/services/src/pro/util/tests.rs index 7936b788b..bb7576cd4 100644 --- a/services/src/pro/util/tests.rs +++ b/services/src/pro/util/tests.rs @@ -450,6 +450,14 @@ where } } +#[cfg(test)] +#[allow(clippy::missing_panics_doc)] +pub async fn get_db_timestamp(app_ctx: &ProPostgresContext) -> DateTime { + let conn = app_ctx.pool.get().await.unwrap(); + let get_time_stmt = conn.prepare("SELECT CURRENT_TIMESTAMP;").await.unwrap(); + conn.query_one(&get_time_stmt, &[]).await.unwrap().get(0) +} + #[cfg(test)] pub(in crate::pro) mod mock_oidc { use crate::pro::users::{DefaultJsonWebKeySet, DefaultProviderMetadata}; diff --git a/services/src/util/tests.rs b/services/src/util/tests.rs index c72b033c3..85383c9eb 100644 --- a/services/src/util/tests.rs +++ b/services/src/util/tests.rs @@ -589,13 +589,3 @@ where Err(err) => std::panic::resume_unwind(err), } } - -#[cfg(test)] -#[allow(clippy::missing_panics_doc)] -pub async fn get_db_timestamp( - app_ctx: &crate::pro::contexts::ProPostgresContext, -) -> geoengine_datatypes::primitives::DateTime { - let conn = app_ctx.pool.get().await.unwrap(); - let get_time_stmt = conn.prepare("SELECT CURRENT_TIMESTAMP;").await.unwrap(); - conn.query_one(&get_time_stmt, &[]).await.unwrap().get(0) -} From 7b4fa9c2b0233db64d3505387f34555d02c2b069 Mon Sep 17 00:00:00 2001 From: Nikolaus Glombiewski Date: Tue, 9 Jul 2024 16:23:46 +0200 Subject: [PATCH 03/17] Remove option to keep data during expiration; Move transaction methods to separate trait; --- .../contexts/migrations/current_schema.sql | 8 +- ...migration_0010_delete_uploaded_datasets.rs | 8 +- services/src/pro/datasets/postgres.rs | 1057 ++++++++--------- services/src/pro/datasets/storage.rs | 130 +- 4 files changed, 591 insertions(+), 612 deletions(-) diff --git a/services/src/pro/contexts/migrations/current_schema.sql b/services/src/pro/contexts/migrations/current_schema.sql index 77baf797f..a9bfed7a0 100644 --- a/services/src/pro/contexts/migrations/current_schema.sql +++ b/services/src/pro/contexts/migrations/current_schema.sql @@ -237,6 +237,11 @@ CREATE TYPE "InternalUploadedDatasetStatus" AS ENUM ( 'DeletedWithError' ); +CREATE TYPE "DatasetDeletionType" AS ENUM ( + 'DeleteRecordAndData', + 'DeleteData' +); + CREATE TABLE uploaded_user_datasets ( user_id uuid, upload_id uuid, @@ -245,7 +250,6 @@ CREATE TABLE uploaded_user_datasets ( created timestamp with time zone NOT NULL, expiration timestamp with time zone, deleted timestamp with time zone, - delete_data boolean NOT NULL, - delete_record boolean NOT NULL, + deletion_type "DatasetDeletionType", PRIMARY KEY (user_id, dataset_id, upload_id) ); diff --git a/services/src/pro/contexts/migrations/migration_0010_delete_uploaded_datasets.rs b/services/src/pro/contexts/migrations/migration_0010_delete_uploaded_datasets.rs index 5792e9eaa..a166aa5cf 100644 --- a/services/src/pro/contexts/migrations/migration_0010_delete_uploaded_datasets.rs +++ b/services/src/pro/contexts/migrations/migration_0010_delete_uploaded_datasets.rs @@ -18,6 +18,11 @@ impl ProMigration for ProMigrationImpl { 'DeletedWithError' ); + CREATE TYPE "DatasetDeletionType" AS ENUM ( + 'DeleteRecordAndData', + 'DeleteData' + ); + CREATE TABLE uploaded_user_datasets ( user_id uuid, upload_id uuid, @@ -26,8 +31,7 @@ impl ProMigration for ProMigrationImpl { created timestamp with time zone NOT NULL, expiration timestamp with time zone, deleted timestamp with time zone, - delete_data boolean NOT NULL, - delete_record boolean NOT NULL, + deletion_type "DatasetDeletionType", PRIMARY KEY (user_id, dataset_id, upload_id) ); "#, diff --git a/services/src/pro/datasets/postgres.rs b/services/src/pro/datasets/postgres.rs index e2f9bf569..126449423 100644 --- a/services/src/pro/datasets/postgres.rs +++ b/services/src/pro/datasets/postgres.rs @@ -12,9 +12,11 @@ use crate::error::Error::{ }; use crate::error::{self, Error, Result}; use crate::pro::contexts::ProPostgresDb; +use crate::pro::datasets::storage::DatasetDeletionType::{DeleteData, DeleteRecordAndData}; +use crate::pro::datasets::storage::InternalUploadedDatasetStatus::{Deleted, DeletedWithError}; use crate::pro::datasets::storage::{ - ChangeDatasetExpiration, InternalUploadedDatasetStatus, UploadedDatasetStatus, - UploadedUserDatasetStore, + ChangeDatasetExpiration, DatasetDeletionType, InternalUploadedDatasetStatus, + TxUploadedUserDatasetStore, UploadedDatasetStatus, UploadedUserDatasetStore, }; use crate::pro::datasets::{Expiration, ExpirationChange}; use crate::pro::permissions::postgres_permissiondb::TxPermissionDb; @@ -38,6 +40,7 @@ use geoengine_operators::source::{GdalLoadingInfo, OgrSourceDataset}; use postgres_types::{FromSql, ToSql}; use snafu::ensure; use tokio_postgres::Transaction; +use InternalUploadedDatasetStatus::{Available, Expired, Expires, UpdateExpired}; impl DatasetDb for ProPostgresDb where @@ -793,15 +796,8 @@ where } } - let expire_dataset = ChangeDatasetExpiration { - dataset_id, - expiration_change: ExpirationChange::SetExpire(Expiration { - deletion_timestamp: None, - delete_record: true, - delete_data: false, - }), - }; - self.expire_uploaded_dataset(expire_dataset).await?; + self.expire_uploaded_dataset(ChangeDatasetExpiration::delete_full(dataset_id)) + .await?; Ok(()) } @@ -912,6 +908,445 @@ impl From for crate::datasets::upload::FileUpload { } } +#[async_trait] +impl TxUploadedUserDatasetStore for ProPostgresDb +where + Tls: MakeTlsConnect + Clone + Send + Sync + 'static + std::fmt::Debug, + >::Stream: Send + Sync, + >::TlsConnect: Send, + <>::TlsConnect as TlsConnect>::Future: Send, +{ + async fn validate_expiration_request_in_tx( + &self, + tx: &Transaction, + dataset_id: &DatasetId, + expiration: &Expiration, + ) -> Result<()> { + let (status, deletion_type, legal_expiration): ( + InternalUploadedDatasetStatus, + Option, + bool, + ) = if let Some(timestamp) = expiration.deletion_timestamp { + let stmt = tx + .prepare( + " + SELECT + status, + deletion_type, + $2 >= CURRENT_TIMESTAMP as legal_expiration + FROM + uploaded_user_datasets + WHERE + dataset_id = $1;", + ) + .await?; + let row = tx + .query_opt(&stmt, &[&dataset_id, ×tamp]) + .await? + .ok_or(UnknownDatasetId)?; + (row.get(0), row.get(1), row.get::(2)) + } else { + let stmt = tx + .prepare( + " + SELECT + status, + deletion_type, + TRUE as legal_expiration + FROM + uploaded_user_datasets + WHERE + dataset_id = $1;", + ) + .await?; + let row = tx + .query_opt(&stmt, &[&dataset_id]) + .await? + .ok_or(UnknownDatasetId)?; + (row.get(0), row.get(1), row.get::(2)) + }; + + match status { + Available | Expires => { + if !legal_expiration { + return Err(ExpirationTimestampInPast); + } + } + Expired | UpdateExpired | Deleted => { + if matches!(expiration.deletion_type, DeleteData) + && matches!(deletion_type, Some(DeleteRecordAndData)) + { + return Err(IllegalExpirationUpdate { + reason: "Prior expiration already deleted data and record".to_string(), + }); + } + if expiration.deletion_timestamp.is_some() { + return Err(IllegalExpirationUpdate { + reason: "Setting expiration after deletion".to_string(), + }); + } + } + DeletedWithError => { + return Err(IllegalDatasetStatus { + status: "Dataset was deleted, but an error occurred during deletion" + .to_string(), + }); + } + } + Ok(()) + } + + async fn uploaded_dataset_status_in_tx( + &self, + dataset_id: &DatasetId, + tx: &Transaction, + ) -> Result { + self.ensure_permission_in_tx((*dataset_id).into(), Permission::Read, tx) + .await + .boxed_context(crate::error::PermissionDb)?; + + self.update_dataset_status_in_tx(tx, dataset_id).await?; + + let stmt = tx + .prepare( + " + SELECT + status + FROM + uploaded_user_datasets + WHERE + dataset_id = $1;", + ) + .await?; + + let result = tx + .query_opt(&stmt, &[&dataset_id]) + .await? + .ok_or(error::Error::UnknownDatasetId)?; + + let internal_status: InternalUploadedDatasetStatus = result.get(0); + + Ok(internal_status.into()) + } + + async fn update_dataset_status_in_tx( + &self, + tx: &Transaction, + dataset_id: &DatasetId, + ) -> Result<()> { + let mut newly_expired_datasets = 0; + + let delete_records = tx + .prepare( + " + DELETE FROM + datasets + USING + user_permitted_datasets p, uploaded_user_datasets u + WHERE + p.user_id = $1 AND datasets.id = $2 + AND p.dataset_id = datasets.id AND u.dataset_id = datasets.id + AND (u.status = $3 OR u.status = $4) AND u.expiration <= CURRENT_TIMESTAMP + AND u.deletion_type = $5;", + ) + .await?; + newly_expired_datasets += tx + .execute( + &delete_records, + &[ + &self.session.user.id, + &dataset_id, + &Expires, + &UpdateExpired, + &DeleteRecordAndData, + ], + ) + .await?; + + let tag_deletion = tx + .prepare( + " + UPDATE + datasets + SET + tags = tags || '{deleted}' + FROM + user_permitted_datasets p, uploaded_user_datasets u + WHERE + p.user_id = $1 AND datasets.id = $2 + AND p.dataset_id = datasets.id AND u.dataset_id = datasets.id + AND u.status = $3 AND u.expiration <= CURRENT_TIMESTAMP + AND u.deletion_type = $4;", + ) + .await?; + newly_expired_datasets += tx + .execute( + &tag_deletion, + &[&self.session.user.id, &dataset_id, &Expires, &DeleteData], + ) + .await?; + + if newly_expired_datasets > 0 { + let mark_deletion = tx + .prepare( + " + UPDATE + uploaded_user_datasets + SET + status = $3 + FROM + user_permitted_datasets p + WHERE + p.user_id = $1 AND uploaded_user_datasets.dataset_id = $2 + AND uploaded_user_datasets.dataset_id = p.dataset_id + AND (status = $4 OR status = $5) AND expiration <= CURRENT_TIMESTAMP;", + ) + .await?; + + tx.execute( + &mark_deletion, + &[ + &self.session.user.id, + &dataset_id, + &Expired, + &Expires, + &UpdateExpired, + ], + ) + .await?; + } + + Ok(()) + } + + async fn update_datasets_status_in_tx(&self, tx: &Transaction) -> Result<()> { + if self.session.is_admin() { + self.admin_update_datasets_status_in_tx(tx).await?; + } else { + let delete_records = tx + .prepare( + " + DELETE FROM + datasets + USING + user_permitted_datasets p, uploaded_user_datasets u + WHERE + p.user_id = $1 AND p.dataset_id = datasets.id AND u.dataset_id = datasets.id + AND u.status = $2 AND u.expiration <= CURRENT_TIMESTAMP + AND u.deletion_type = $3;", + ) + .await?; + tx.execute( + &delete_records, + &[&self.session.user.id, &Expires, &DeleteRecordAndData], + ) + .await?; + + let tag_deletion = tx + .prepare( + " + UPDATE + datasets + SET + tags = tags || '{deleted}' + FROM + user_permitted_datasets p, uploaded_user_datasets u + WHERE + p.user_id = $1 AND p.dataset_id = datasets.id AND u.dataset_id = datasets.id + AND u.status = $2 AND u.expiration <= CURRENT_TIMESTAMP + AND u.deletion_type = $3;", + ) + .await?; + tx.execute( + &tag_deletion, + &[&self.session.user.id, &Expires, &DeleteData], + ) + .await?; + + let mark_deletion = tx + .prepare( + " + UPDATE + uploaded_user_datasets + SET + status = $2 + FROM + user_permitted_datasets p + WHERE + p.user_id = $1 AND uploaded_user_datasets.dataset_id = p.dataset_id + AND (status = $3 OR status = $4) AND expiration <= CURRENT_TIMESTAMP;", + ) + .await?; + + tx.execute( + &mark_deletion, + &[&self.session.user.id, &Expired, &Expires, &UpdateExpired], + ) + .await?; + } + Ok(()) + } + + async fn admin_update_datasets_status_in_tx(&self, tx: &Transaction) -> Result<()> { + ensure!(self.session.is_admin(), error::PermissionDenied); + let delete_records = tx + .prepare( + " + DELETE FROM + datasets + USING + user_permitted_datasets p, uploaded_user_datasets u + WHERE + u.dataset_id = datasets.id + AND u.status = $1 AND u.expiration <= CURRENT_TIMESTAMP + AND u.deletion_type = $2;", + ) + .await?; + tx.execute(&delete_records, &[&Expires, &DeleteRecordAndData]) + .await?; + + let tag_deletion = tx + .prepare( + " + UPDATE + datasets + SET + tags = tags || '{deleted}' + FROM + uploaded_user_datasets u + WHERE + u.dataset_id = datasets.id + AND u.status = $1 AND u.expiration <= CURRENT_TIMESTAMP + AND u.deletion_type = $2;", + ) + .await?; + tx.execute(&tag_deletion, &[&Expires, &DeleteData]).await?; + + let mark_deletion = tx + .prepare( + " + UPDATE + uploaded_user_datasets + SET + status = $1 + WHERE + (status = $2 or status = $3) AND expiration <= CURRENT_TIMESTAMP;", + ) + .await?; + + tx.execute(&mark_deletion, &[&Expired, &Expires, &UpdateExpired]) + .await?; + Ok(()) + } + + async fn set_expire_for_uploaded_dataset( + &self, + tx: &Transaction, + dataset_id: &DatasetId, + expiration: &Expiration, + ) -> Result<()> { + let num_changes = if let Some(delete_timestamp) = expiration.deletion_timestamp { + let stmt = tx + .prepare(" + UPDATE uploaded_user_datasets + SET status = $2, expiration = $3, deletion_type = $4 + WHERE dataset_id = $1 AND $3 >= CURRENT_TIMESTAMP AND (status = $5 OR status = $6);", + ).await?; + tx.execute( + &stmt, + &[ + &dataset_id, + &Expires, + &delete_timestamp, + &expiration.deletion_type, + &Available, + &Expires, + ], + ) + .await? + } else { + let stmt = tx + .prepare( + " + UPDATE uploaded_user_datasets + SET status = $2, expiration = CURRENT_TIMESTAMP, deletion_type = $3 + WHERE dataset_id = $1 AND (status = $4 OR status = $5);", + ) + .await?; + let num_expired = tx + .execute( + &stmt, + &[ + &dataset_id, + &Expires, + &expiration.deletion_type, + &Available, + &Expires, + ], + ) + .await?; + + if num_expired == 0 && matches!(expiration.deletion_type, DeleteRecordAndData) { + let stmt = tx + .prepare( + " + UPDATE uploaded_user_datasets + SET deletion_type = $2, + status = $3 + WHERE dataset_id = $1 AND (status = $4 OR status = $5) AND deletion_type = $6;", + ) + .await?; + tx.execute( + &stmt, + &[ + &dataset_id, + &expiration.deletion_type, + &UpdateExpired, + &Expired, + &Deleted, + &DeleteData, + ], + ) + .await? + } else { + num_expired + } + }; + + if num_changes == 0 { + self.validate_expiration_request_in_tx(tx, dataset_id, expiration) + .await?; + }; + + Ok(()) + } + + async fn unset_expire_for_uploaded_dataset( + &self, + tx: &Transaction, + dataset_id: &DatasetId, + ) -> Result<()> { + let stmt = tx + .prepare( + " + UPDATE uploaded_user_datasets + SET status = $2, expiration = NULL, deletion_type = NULL + WHERE dataset_id = $1 AND status = $3;", + ) + .await?; + let set_changes = tx + .execute(&stmt, &[&dataset_id, &Available, &Expires]) + .await?; + if set_changes == 0 { + return Err(IllegalDatasetStatus { + status: "Requested dataset does not exist or does not have an expiration" + .to_string(), + }); + } + Ok(()) + } +} + #[async_trait] impl UploadedUserDatasetStore for ProPostgresDb where @@ -979,473 +1414,100 @@ where &dataset.provenance, &dataset.tags, ], - ) - .await - .map_unique_violation("datasets", "name", || error::Error::InvalidDatasetName)?; - - let stmt = tx - .prepare( - " - INSERT INTO permissions ( - role_id, - dataset_id, - permission - ) - VALUES ($1, $2, $3)", - ) - .await?; - - tx.execute( - &stmt, - &[&RoleId::from(self.session.user.id), &id, &Permission::Owner], - ) - .await?; - - let stmt = tx - .prepare( - " - INSERT INTO uploaded_user_datasets ( - user_id, - upload_id, - dataset_id, - status, - created, - expiration, - deleted, - delete_data, - delete_record - ) - VALUES ($1, $2, $3, 'Available', CURRENT_TIMESTAMP, NULL, NULL, false, false)", - ) - .await?; - - tx.execute( - &stmt, - &[&RoleId::from(self.session.user.id), &upload_id, &id], - ) - .await?; - - tx.commit().await?; - - Ok(DatasetIdAndName { id, name }) - } - - async fn expire_uploaded_dataset(&self, expire_dataset: ChangeDatasetExpiration) -> Result<()> { - let mut conn = self.conn_pool.get().await?; - let tx = conn.build_transaction().start().await?; - - self.ensure_permission_in_tx(expire_dataset.dataset_id.into(), Permission::Owner, &tx) - .await - .boxed_context(crate::error::PermissionDb)?; - - self.update_dataset_status_in_tx(&tx, &expire_dataset.dataset_id) - .await?; - - match expire_dataset.expiration_change { - ExpirationChange::SetExpire(expiration) => { - let num_changes = if let Some(delete_timestamp) = expiration.deletion_timestamp { - let stmt = tx - .prepare(" - UPDATE uploaded_user_datasets - SET status = 'Expires', expiration = $2, delete_data = $3, delete_record = $4 - WHERE dataset_id = $1 AND $2 >= CURRENT_TIMESTAMP AND (status = 'Available' OR status = 'Expires');", - ).await?; - tx.execute( - &stmt, - &[ - &expire_dataset.dataset_id, - &delete_timestamp, - &expiration.delete_data, - &expiration.delete_record, - ], - ) - .await? - } else { - let stmt = tx.prepare(" - UPDATE uploaded_user_datasets - SET status = 'Expires', expiration = CURRENT_TIMESTAMP, delete_data = $2, delete_record = $3 - WHERE dataset_id = $1 AND (status = 'Available' OR status = 'Expires');", - ).await?; - let num_expired = tx - .execute( - &stmt, - &[ - &expire_dataset.dataset_id, - &expiration.delete_data, - &expiration.delete_record, - ], - ) - .await?; - - if num_expired == 0 { - let stmt = tx - .prepare( - " - UPDATE uploaded_user_datasets - SET delete_data = (CASE WHEN NOT delete_data THEN $2 ELSE true END), - delete_record = (CASE WHEN NOT delete_record THEN $3 ELSE true END), - status = 'UpdateExpired' - WHERE dataset_id = $1 AND (status = 'Expired' OR status = 'Deleted') - AND (delete_data = false OR delete_record = false);", - ) - .await?; - tx.execute( - &stmt, - &[ - &expire_dataset.dataset_id, - &expiration.delete_data, - &expiration.delete_record, - ], - ) - .await? - } else { - num_expired - } - }; - - if num_changes == 0 { - self.validate_expiration_request_in_tx( - &tx, - &expire_dataset.dataset_id, - &expiration, - ) - .await?; - }; - } - ExpirationChange::UnsetExpire => { - let stmt = tx.prepare(" - UPDATE uploaded_user_datasets - SET status = 'Available', expiration = NULL, delete_data = false, delete_record = false - WHERE dataset_id = $1 AND status = 'Expires';", - ).await?; - let set_changes = tx.execute(&stmt, &[&expire_dataset.dataset_id]).await?; - if set_changes == 0 { - return Err(IllegalDatasetStatus { - status: "Requested dataset does not exist or does not have an expiration" - .to_string(), - }); - } - } - } - self.update_dataset_status_in_tx(&tx, &expire_dataset.dataset_id) - .await?; - - tx.commit().await?; - - Ok(()) - } - - async fn validate_expiration_request_in_tx( - &self, - tx: &Transaction, - dataset_id: &DatasetId, - expiration: &Expiration, - ) -> Result<()> { - let (status, delete_data, delete_record, legal_expiration) = - if let Some(timestamp) = expiration.deletion_timestamp { - let stmt = tx - .prepare( - " - SELECT - status, - delete_data, - delete_record, - $2 >= CURRENT_TIMESTAMP as legal_expiration - FROM - uploaded_user_datasets - WHERE - dataset_id = $1;", - ) - .await?; - let row = tx - .query_opt(&stmt, &[&dataset_id, ×tamp]) - .await? - .ok_or(UnknownDatasetId)?; - ( - row.get(0), - row.get(1), - row.get(2), - row.get::(3), - ) - } else { - let stmt = tx - .prepare( - " - SELECT - status, - delete_data, - delete_record, - TRUE as legal_expiration - FROM - uploaded_user_datasets - WHERE - dataset_id = $1;", - ) - .await?; - let row = tx - .query_opt(&stmt, &[&dataset_id]) - .await? - .ok_or(UnknownDatasetId)?; - ( - row.get(0), - row.get(1), - row.get(2), - row.get::(3), - ) - }; - - match status { - InternalUploadedDatasetStatus::Available | InternalUploadedDatasetStatus::Expires => { - if !legal_expiration { - return Err(ExpirationTimestampInPast); - } - } - InternalUploadedDatasetStatus::Expired - | InternalUploadedDatasetStatus::UpdateExpired - | InternalUploadedDatasetStatus::Deleted => { - let data_downgrade = delete_data && !expiration.delete_data; - let record_downgrade = delete_record && !expiration.delete_record; - if data_downgrade || record_downgrade { - let data = if data_downgrade { " data" } else { "" }; - let record = if record_downgrade { " record" } else { "" }; - return Err(IllegalExpirationUpdate { - reason: format!("Prior expiration deleted: {data} {record}"), - }); - } - if expiration.deletion_timestamp.is_some() { - return Err(IllegalExpirationUpdate { - reason: "Setting expiration after deletion".to_string(), - }); - } - } - InternalUploadedDatasetStatus::DeletedWithError => { - return Err(IllegalDatasetStatus { - status: "Dataset was deleted, but an error occurred during deletion" - .to_string(), - }); - } - } - Ok(()) - } - - async fn uploaded_dataset_status_in_tx( - &self, - dataset_id: &DatasetId, - tx: &Transaction, - ) -> Result { - self.ensure_permission_in_tx((*dataset_id).into(), Permission::Read, tx) - .await - .boxed_context(crate::error::PermissionDb)?; + ) + .await + .map_unique_violation("datasets", "name", || error::Error::InvalidDatasetName)?; - self.update_dataset_status_in_tx(tx, dataset_id).await?; + let stmt = tx + .prepare( + " + INSERT INTO permissions ( + role_id, + dataset_id, + permission + ) + VALUES ($1, $2, $3)", + ) + .await?; + + tx.execute( + &stmt, + &[&RoleId::from(self.session.user.id), &id, &Permission::Owner], + ) + .await?; let stmt = tx .prepare( " - SELECT - status - FROM - uploaded_user_datasets - WHERE - dataset_id = $1;", + INSERT INTO uploaded_user_datasets ( + user_id, + upload_id, + dataset_id, + status, + created, + expiration, + deleted, + deletion_type + ) + VALUES ($1, $2, $3, 'Available', CURRENT_TIMESTAMP, NULL, NULL, NULL)", ) .await?; - let result = tx - .query_opt(&stmt, &[&dataset_id]) - .await? - .ok_or(error::Error::UnknownDatasetId)?; + tx.execute( + &stmt, + &[&RoleId::from(self.session.user.id), &upload_id, &id], + ) + .await?; - let internal_status: InternalUploadedDatasetStatus = result.get(0); + tx.commit().await?; - Ok(internal_status.into()) + Ok(DatasetIdAndName { id, name }) } - async fn update_dataset_status(&self, dataset_id: &DatasetId) -> Result<()> { + async fn expire_uploaded_dataset(&self, expire_dataset: ChangeDatasetExpiration) -> Result<()> { let mut conn = self.conn_pool.get().await?; let tx = conn.build_transaction().start().await?; - self.update_dataset_status_in_tx(&tx, dataset_id).await?; - tx.commit().await?; - - Ok(()) - } - async fn update_dataset_status_in_tx( - &self, - tx: &Transaction, - dataset_id: &DatasetId, - ) -> Result<()> { - let mut newly_expired_datasets = 0; + self.ensure_permission_in_tx(expire_dataset.dataset_id.into(), Permission::Owner, &tx) + .await + .boxed_context(error::PermissionDb)?; - let delete_records = tx - .prepare( - " - DELETE FROM - datasets - USING - user_permitted_datasets p, uploaded_user_datasets u - WHERE - p.user_id = $1 AND datasets.id = $2 - AND p.dataset_id = datasets.id AND u.dataset_id = datasets.id - AND (u.status = 'Expires' OR u.status = 'UpdateExpired') AND u.expiration <= CURRENT_TIMESTAMP - AND u.delete_record;", - ) - .await?; - newly_expired_datasets += tx - .execute(&delete_records, &[&self.session.user.id, &dataset_id]) + self.update_dataset_status_in_tx(&tx, &expire_dataset.dataset_id) .await?; - let tag_deletion = tx - .prepare( - " - UPDATE - datasets - SET - tags = tags || '{deleted}' - FROM - user_permitted_datasets p, uploaded_user_datasets u - WHERE - p.user_id = $1 AND datasets.id = $2 - AND p.dataset_id = datasets.id AND u.dataset_id = datasets.id - AND u.status = 'Expires' AND u.expiration <= CURRENT_TIMESTAMP - AND NOT u.delete_record;", - ) - .await?; - newly_expired_datasets += tx - .execute(&tag_deletion, &[&self.session.user.id, &dataset_id]) + match expire_dataset.expiration_change { + ExpirationChange::SetExpire(expiration) => { + self.set_expire_for_uploaded_dataset(&tx, &expire_dataset.dataset_id, &expiration) + .await?; + } + ExpirationChange::UnsetExpire => { + self.unset_expire_for_uploaded_dataset(&tx, &expire_dataset.dataset_id) + .await?; + } + } + self.update_dataset_status_in_tx(&tx, &expire_dataset.dataset_id) .await?; - if newly_expired_datasets > 0 { - let mark_deletion = tx - .prepare( - " - UPDATE - uploaded_user_datasets - SET - status = 'Expired' - FROM - user_permitted_datasets p - WHERE - p.user_id = $1 AND uploaded_user_datasets.dataset_id = $2 - AND uploaded_user_datasets.dataset_id = p.dataset_id - AND (status = 'Expires' OR status = 'UpdateExpired') AND expiration <= CURRENT_TIMESTAMP;", - ) - .await?; - - tx.execute(&mark_deletion, &[&self.session.user.id, &dataset_id]) - .await?; - } + tx.commit().await?; Ok(()) } - async fn update_datasets_status(&self) -> Result<()> { + async fn update_dataset_status(&self, dataset_id: &DatasetId) -> Result<()> { let mut conn = self.conn_pool.get().await?; let tx = conn.build_transaction().start().await?; - self.update_datasets_status_in_tx(&tx).await?; + self.update_dataset_status_in_tx(&tx, dataset_id).await?; tx.commit().await?; Ok(()) } - async fn update_datasets_status_in_tx(&self, tx: &Transaction) -> Result<()> { - if self.session.is_admin() { - let delete_records = tx - .prepare( - " - DELETE FROM - datasets - USING - user_permitted_datasets p, uploaded_user_datasets u - WHERE - u.dataset_id = datasets.id - AND u.status = 'Expires' AND u.expiration <= CURRENT_TIMESTAMP - AND u.delete_record;", - ) - .await?; - tx.execute(&delete_records, &[]).await?; - - let tag_deletion = tx - .prepare( - " - UPDATE - datasets - SET - tags = tags || '{deleted}' - FROM - uploaded_user_datasets u - WHERE - u.dataset_id = datasets.id - AND u.status = 'Expires' AND u.expiration <= CURRENT_TIMESTAMP - AND NOT u.delete_record;", - ) - .await?; - tx.execute(&tag_deletion, &[]).await?; - - let mark_deletion = tx - .prepare( - " - UPDATE - uploaded_user_datasets - SET - status = 'Expired' - WHERE - (status = 'Expires' or status = 'UpdateExpired') AND expiration <= CURRENT_TIMESTAMP;", - ) - .await?; - - tx.execute(&mark_deletion, &[]).await?; - } else { - let delete_records = tx - .prepare( - " - DELETE FROM - datasets - USING - user_permitted_datasets p, uploaded_user_datasets u - WHERE - p.user_id = $1 AND p.dataset_id = datasets.id AND u.dataset_id = datasets.id - AND u.status = 'Expires' AND u.expiration <= CURRENT_TIMESTAMP - AND u.delete_record;", - ) - .await?; - tx.execute(&delete_records, &[&self.session.user.id]) - .await?; - - let tag_deletion = tx - .prepare( - " - UPDATE - datasets - SET - tags = tags || '{deleted}' - FROM - user_permitted_datasets p, uploaded_user_datasets u - WHERE - p.user_id = $1 AND p.dataset_id = datasets.id AND u.dataset_id = datasets.id - AND u.status = 'Expires' AND u.expiration <= CURRENT_TIMESTAMP - AND NOT u.delete_record;", - ) - .await?; - tx.execute(&tag_deletion, &[&self.session.user.id]).await?; - - let mark_deletion = tx - .prepare( - " - UPDATE - uploaded_user_datasets - SET - status = 'Expired' - FROM - user_permitted_datasets p - WHERE - p.user_id = $1 AND uploaded_user_datasets.dataset_id = p.dataset_id - AND (status = 'Expires' OR status = 'UpdateExpired') AND expiration <= CURRENT_TIMESTAMP;", - ) - .await?; + async fn update_datasets_status(&self) -> Result<()> { + let mut conn = self.conn_pool.get().await?; + let tx = conn.build_transaction().start().await?; + self.update_datasets_status_in_tx(&tx).await?; + tx.commit().await?; - tx.execute(&mark_deletion, &[&self.session.user.id]).await?; - } Ok(()) } @@ -1465,11 +1527,11 @@ where FROM uploaded_user_datasets WHERE - status = 'Expired' AND delete_data;", + status = $1 AND deletion_type IS NOT NULL;", ) .await?; - let rows = tx.query(&marked_datasets, &[]).await?; + let rows = tx.query(&marked_datasets, &[&Expired]).await?; let mut deleted = vec![]; let mut deleted_with_error = vec![]; @@ -1493,12 +1555,13 @@ where UPDATE uploaded_user_datasets SET - status = 'Deleted' + status = $1 WHERE - status = 'Expired' AND delete_data AND upload_id = ANY($1);", + status = $2 AND deletion_type IS NOT NULL AND upload_id = ANY($3);", ) .await?; - tx.execute(&mark_deletion, &[&deleted]).await?; + tx.execute(&mark_deletion, &[&Deleted, &Expired, &deleted]) + .await?; } if !deleted_with_error.is_empty() { @@ -1508,12 +1571,16 @@ where UPDATE uploaded_user_datasets SET - status = 'DeletedWithError' + status = $1 WHERE - status = 'Expired' AND delete_data AND upload_id = ANY($1);", + status = $2 AND deletion_type IS NOT NULL AND upload_id = ANY($3);", ) .await?; - tx.execute(&mark_error, &[&deleted_with_error]).await?; + tx.execute( + &mark_error, + &[&DeletedWithError, &Expired, &deleted_with_error], + ) + .await?; } tx.commit().await?; @@ -1869,8 +1936,6 @@ mod tests { upload_and_add_point_dataset(&app_ctx, &user_session, "fair", &mut test_data).await; let full = upload_and_add_point_dataset(&app_ctx, &user_session, "full", &mut test_data).await; - let none = - upload_and_add_point_dataset(&app_ctx, &user_session, "none", &mut test_data).await; let db = app_ctx.session_context(user_session.clone()).db(); @@ -1887,18 +1952,14 @@ mod tests { .await .unwrap(); - assert_eq!(listing.len(), 4); + assert_eq!(listing.len(), 3); assert!(listing_not_deleted(listing.first().unwrap(), &available)); assert!(listing_not_deleted(listing.get(1).unwrap(), &fair)); assert!(listing_not_deleted(listing.get(2).unwrap(), &full)); - assert!(listing_not_deleted(listing.get(3).unwrap(), &none)); db.expire_uploaded_dataset(ChangeDatasetExpiration::delete_fair(fair.dataset_id)) .await .unwrap(); - db.expire_uploaded_dataset(ChangeDatasetExpiration::delete_none(none.dataset_id)) - .await - .unwrap(); db.expire_uploaded_dataset(ChangeDatasetExpiration::delete_full(full.dataset_id)) .await .unwrap(); @@ -1918,15 +1979,10 @@ mod tests { db.load_dataset(&full.dataset_id).await.unwrap_err(), UnknownDatasetId )); - assert!(dataset_deleted( - &db.load_dataset(&none.dataset_id).await.unwrap(), - &none - )); assert!(dir_exists(&available)); assert!(dir_exists(&fair)); assert!(dir_exists(&full)); - assert!(dir_exists(&none)); let admin_session = admin_login(&app_ctx).await; let admin_ctx = app_ctx.session_context(admin_session.clone()); @@ -1936,7 +1992,6 @@ mod tests { assert!(dir_exists(&available)); assert!(!dir_exists(&fair)); assert!(!dir_exists(&full)); - assert!(dir_exists(&none)); let deleted = admin_ctx.db().clear_expired_datasets().await.unwrap(); assert_eq!(deleted, 0); @@ -2035,24 +2090,6 @@ mod tests { )) .await .unwrap(); - db.expire_uploaded_dataset(ChangeDatasetExpiration::expire_none( - fair2full.dataset_id, - future_time_1, - )) - .await - .unwrap(); - db.expire_uploaded_dataset(ChangeDatasetExpiration::expire_meta( - fair2full.dataset_id, - future_time_1, - )) - .await - .unwrap(); - db.expire_uploaded_dataset(ChangeDatasetExpiration::expire_fair( - fair2full.dataset_id, - future_time_1, - )) - .await - .unwrap(); db.expire_uploaded_dataset(ChangeDatasetExpiration::expire_full( fair2full.dataset_id, future_time_1, @@ -2129,50 +2166,6 @@ mod tests { UnknownDatasetId )); - let none2fair = - upload_and_add_point_dataset(&app_ctx, &user_session, "none2fair", &mut test_data) - .await; - db.expire_uploaded_dataset(ChangeDatasetExpiration::delete_none(none2fair.dataset_id)) - .await - .unwrap(); - assert!(dataset_deleted( - &db.load_dataset(&none2fair.dataset_id).await.unwrap(), - &none2fair - )); - db.expire_uploaded_dataset(ChangeDatasetExpiration::delete_fair(none2fair.dataset_id)) - .await - .unwrap(); - assert!(dataset_deleted( - &db.load_dataset(&none2fair.dataset_id).await.unwrap(), - &none2fair - )); - - let none2meta = - upload_and_add_point_dataset(&app_ctx, &user_session, "none2meta", &mut test_data) - .await; - db.expire_uploaded_dataset(ChangeDatasetExpiration::delete_none(none2meta.dataset_id)) - .await - .unwrap(); - assert!(dataset_deleted( - &db.load_dataset(&none2meta.dataset_id).await.unwrap(), - &none2meta - )); - db.expire_uploaded_dataset(ChangeDatasetExpiration::delete_meta(none2meta.dataset_id)) - .await - .unwrap(); - assert!(matches!( - db.load_dataset(&none2meta.dataset_id).await.unwrap_err(), - UnknownDatasetId - )); - - assert!(db - .expire_uploaded_dataset(ChangeDatasetExpiration::delete_none(none2fair.dataset_id)) - .await - .is_err()); - assert!(db - .expire_uploaded_dataset(ChangeDatasetExpiration::delete_none(fair2full.dataset_id)) - .await - .is_err()); assert!(db .expire_uploaded_dataset(ChangeDatasetExpiration::delete_fair(fair2full.dataset_id)) .await @@ -2200,7 +2193,7 @@ mod tests { let admin_session = admin_login(&app_ctx).await; let admin_ctx = app_ctx.session_context(admin_session.clone()); let deleted = admin_ctx.db().clear_expired_datasets().await.unwrap(); - assert_eq!(deleted, 2); + assert_eq!(deleted, 1); let listing = db .list_datasets(default_list_options.clone()) @@ -2214,8 +2207,6 @@ mod tests { assert!(dir_exists(&fair2available)); assert!(!dir_exists(&fair2full)); - assert!(!dir_exists(&none2fair)); - assert!(dir_exists(&none2meta)); } #[ge_context::test] @@ -2242,7 +2233,7 @@ mod tests { assert!(err.is_err()); assert!(matches!(err.unwrap_err(), ExpirationTimestampInPast)); - //Unset expire for not-expiring dataset + //Unset expire for non-expiring dataset let err = db .expire_uploaded_dataset(ChangeDatasetExpiration::unset_expire( test_dataset.dataset_id, diff --git a/services/src/pro/datasets/storage.rs b/services/src/pro/datasets/storage.rs index eef64a703..72c8c87f6 100644 --- a/services/src/pro/datasets/storage.rs +++ b/services/src/pro/datasets/storage.rs @@ -1,20 +1,28 @@ -use crate::datasets::storage::MetaDataDefinition; -use crate::datasets::upload::UploadId; -use crate::datasets::{AddDataset, DatasetIdAndName}; -use crate::error::Result; use async_trait::async_trait; -use geoengine_datatypes::dataset::DatasetId; -use geoengine_datatypes::primitives::DateTime; use postgres_types::{FromSql, ToSql}; use serde::{Deserialize, Serialize}; use tokio_postgres::Transaction; use utoipa::ToSchema; +use geoengine_datatypes::dataset::DatasetId; +use geoengine_datatypes::primitives::DateTime; + +use crate::datasets::storage::MetaDataDefinition; +use crate::datasets::upload::UploadId; +use crate::datasets::{AddDataset, DatasetIdAndName}; +use crate::error::Result; +use crate::pro::datasets::storage::DatasetDeletionType::{DeleteData, DeleteRecordAndData}; + #[derive(Deserialize, Serialize, Debug, Clone, ToSchema)] pub struct Expiration { pub deletion_timestamp: Option, - pub delete_record: bool, - pub delete_data: bool, + pub deletion_type: DatasetDeletionType, +} + +#[derive(Deserialize, Serialize, Debug, Clone, ToSchema, FromSql, ToSql)] +pub enum DatasetDeletionType { + DeleteRecordAndData, + DeleteData, } #[derive(Deserialize, Serialize, Debug, Clone)] @@ -24,24 +32,12 @@ pub struct ChangeDatasetExpiration { } impl ChangeDatasetExpiration { - pub fn delete_meta(dataset_id: DatasetId) -> Self { - ChangeDatasetExpiration { - dataset_id, - expiration_change: ExpirationChange::SetExpire(Expiration { - deletion_timestamp: None, - delete_record: true, - delete_data: false, - }), - } - } - pub fn delete_fair(dataset_id: DatasetId) -> Self { ChangeDatasetExpiration { dataset_id, expiration_change: ExpirationChange::SetExpire(Expiration { deletion_timestamp: None, - delete_record: false, - delete_data: true, + deletion_type: DeleteData, }), } } @@ -51,30 +47,7 @@ impl ChangeDatasetExpiration { dataset_id, expiration_change: ExpirationChange::SetExpire(Expiration { deletion_timestamp: None, - delete_record: true, - delete_data: true, - }), - } - } - - pub fn delete_none(dataset_id: DatasetId) -> Self { - ChangeDatasetExpiration { - dataset_id, - expiration_change: ExpirationChange::SetExpire(Expiration { - deletion_timestamp: None, - delete_record: false, - delete_data: false, - }), - } - } - - pub fn expire_meta(dataset_id: DatasetId, timestamp: DateTime) -> Self { - ChangeDatasetExpiration { - dataset_id, - expiration_change: ExpirationChange::SetExpire(Expiration { - deletion_timestamp: Some(timestamp), - delete_record: true, - delete_data: false, + deletion_type: DeleteRecordAndData, }), } } @@ -84,8 +57,7 @@ impl ChangeDatasetExpiration { dataset_id, expiration_change: ExpirationChange::SetExpire(Expiration { deletion_timestamp: Some(timestamp), - delete_record: false, - delete_data: true, + deletion_type: DeleteData, }), } } @@ -95,19 +67,7 @@ impl ChangeDatasetExpiration { dataset_id, expiration_change: ExpirationChange::SetExpire(Expiration { deletion_timestamp: Some(timestamp), - delete_record: true, - delete_data: true, - }), - } - } - - pub fn expire_none(dataset_id: DatasetId, timestamp: DateTime) -> Self { - ChangeDatasetExpiration { - dataset_id, - expiration_change: ExpirationChange::SetExpire(Expiration { - deletion_timestamp: Some(timestamp), - delete_record: false, - delete_data: false, + deletion_type: DeleteRecordAndData, }), } } @@ -156,18 +116,12 @@ impl From for UploadedDatasetStatus { } } -/// Storage of user-uploaded datasets +/// internal functionality for transactional control of user-uploaded datasets db +/// +/// In contrast to the `UploadedUserDatasetStore` this is not to be used by services but only by the `ProPostgresDb` internally. +/// This is because services do not know about database transactions. #[async_trait] -pub trait UploadedUserDatasetStore { - async fn add_uploaded_dataset( - &self, - upload_id: UploadId, - dataset: AddDataset, - meta_data: MetaDataDefinition, - ) -> Result; - - async fn expire_uploaded_dataset(&self, expire_dataset: ChangeDatasetExpiration) -> Result<()>; - +pub trait TxUploadedUserDatasetStore { async fn validate_expiration_request_in_tx( &self, tx: &Transaction, @@ -181,17 +135,43 @@ pub trait UploadedUserDatasetStore { tx: &Transaction, ) -> Result; - async fn update_dataset_status(&self, dataset_id: &DatasetId) -> Result<()>; - async fn update_dataset_status_in_tx( &self, tx: &Transaction, dataset_id: &DatasetId, ) -> Result<()>; - async fn update_datasets_status(&self) -> Result<()>; - async fn update_datasets_status_in_tx(&self, tx: &Transaction) -> Result<()>; + async fn admin_update_datasets_status_in_tx(&self, tx: &Transaction) -> Result<()>; + async fn set_expire_for_uploaded_dataset( + &self, + tx: &Transaction, + dataset_id: &DatasetId, + expiration: &Expiration, + ) -> Result<()>; + async fn unset_expire_for_uploaded_dataset( + &self, + tx: &Transaction, + dataset_id: &DatasetId, + ) -> Result<()>; +} + +/// Storage of user-uploaded datasets +#[async_trait] +pub trait UploadedUserDatasetStore { + async fn add_uploaded_dataset( + &self, + upload_id: UploadId, + dataset: AddDataset, + meta_data: MetaDataDefinition, + ) -> Result; + + async fn expire_uploaded_dataset(&self, expire_dataset: ChangeDatasetExpiration) -> Result<()>; + + async fn update_dataset_status(&self, dataset_id: &DatasetId) -> Result<()>; + + async fn update_datasets_status(&self) -> Result<()>; + async fn clear_expired_datasets(&self) -> Result; } From c56db3069fa8e7e6bc42f6a2626134a21a82ce95 Mon Sep 17 00:00:00 2001 From: Nikolaus Glombiewski Date: Tue, 9 Jul 2024 17:49:16 +0200 Subject: [PATCH 04/17] Fix missing expiration updates; --- services/src/pro/datasets/postgres.rs | 41 ++++++++++++++++++++------- services/src/pro/datasets/storage.rs | 2 +- 2 files changed, 32 insertions(+), 11 deletions(-) diff --git a/services/src/pro/datasets/postgres.rs b/services/src/pro/datasets/postgres.rs index 126449423..22b2300da 100644 --- a/services/src/pro/datasets/postgres.rs +++ b/services/src/pro/datasets/postgres.rs @@ -1511,7 +1511,7 @@ where Ok(()) } - async fn clear_expired_datasets(&self) -> Result { + async fn clear_expired_datasets(&self) -> Result { ensure!(self.session.is_admin(), error::PermissionDenied); let mut conn = self.conn_pool.get().await?; @@ -1519,6 +1519,19 @@ where self.update_datasets_status_in_tx(&tx).await?; + let update_expired = tx + .prepare( + " + UPDATE + uploaded_user_datasets + SET + status = $1 + WHERE + status = $2 AND deleted IS NOT NULL;", + ) + .await?; + let mut updated = tx.execute(&update_expired, &[&Deleted, &Expired]).await?; + let marked_datasets = tx .prepare( " @@ -1527,7 +1540,7 @@ where FROM uploaded_user_datasets WHERE - status = $1 AND deletion_type IS NOT NULL;", + status = $1 AND deleted IS NULL;", ) .await?; @@ -1546,6 +1559,7 @@ where } else { deleted.push(upload_id); } + updated += 1; //Could hypothetically overflow } if !deleted.is_empty() { @@ -1555,9 +1569,9 @@ where UPDATE uploaded_user_datasets SET - status = $1 + status = $1, deleted = CURRENT_TIMESTAMP WHERE - status = $2 AND deletion_type IS NOT NULL AND upload_id = ANY($3);", + status = $2 AND upload_id = ANY($3);", ) .await?; tx.execute(&mark_deletion, &[&Deleted, &Expired, &deleted]) @@ -1571,9 +1585,9 @@ where UPDATE uploaded_user_datasets SET - status = $1 + status = $1, deleted = CURRENT_TIMESTAMP WHERE - status = $2 AND deletion_type IS NOT NULL AND upload_id = ANY($3);", + status = $2 AND upload_id = ANY($3);", ) .await?; tx.execute( @@ -1585,7 +1599,7 @@ where tx.commit().await?; - Ok(deleted.len()) + Ok(updated) } } @@ -2158,6 +2172,12 @@ mod tests { &db.load_dataset(&fair2full.dataset_id).await.unwrap(), &fair2full )); + + let admin_session = admin_login(&app_ctx).await; + let admin_ctx = app_ctx.session_context(admin_session.clone()); + let deleted = admin_ctx.db().clear_expired_datasets().await.unwrap(); + assert_eq!(deleted, 1); + db.expire_uploaded_dataset(ChangeDatasetExpiration::delete_full(fair2full.dataset_id)) .await .unwrap(); @@ -2166,6 +2186,9 @@ mod tests { UnknownDatasetId )); + let deleted = admin_ctx.db().clear_expired_datasets().await.unwrap(); + assert_eq!(deleted, 1); + assert!(db .expire_uploaded_dataset(ChangeDatasetExpiration::delete_fair(fair2full.dataset_id)) .await @@ -2190,10 +2213,8 @@ mod tests { tokio::time::sleep(std::time::Duration::from_secs(5)).await; - let admin_session = admin_login(&app_ctx).await; - let admin_ctx = app_ctx.session_context(admin_session.clone()); let deleted = admin_ctx.db().clear_expired_datasets().await.unwrap(); - assert_eq!(deleted, 1); + assert_eq!(deleted, 0); let listing = db .list_datasets(default_list_options.clone()) diff --git a/services/src/pro/datasets/storage.rs b/services/src/pro/datasets/storage.rs index 72c8c87f6..c09c5dfdd 100644 --- a/services/src/pro/datasets/storage.rs +++ b/services/src/pro/datasets/storage.rs @@ -173,5 +173,5 @@ pub trait UploadedUserDatasetStore { async fn update_datasets_status(&self) -> Result<()>; - async fn clear_expired_datasets(&self) -> Result; + async fn clear_expired_datasets(&self) -> Result; } From 3fc317a8357d323dec83b88a67c4419ca9dc507f Mon Sep 17 00:00:00 2001 From: Nikolaus Glombiewski Date: Tue, 9 Jul 2024 18:03:54 +0200 Subject: [PATCH 05/17] Fix expiration open-api generation and docs; --- services/src/pro/api/apidoc.rs | 2 ++ services/src/pro/api/handlers/datasets.rs | 8 +++----- services/src/pro/datasets/mod.rs | 3 ++- services/src/pro/datasets/storage.rs | 1 + 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/services/src/pro/api/apidoc.rs b/services/src/pro/api/apidoc.rs index 26b256c81..23d735e84 100644 --- a/services/src/pro/api/apidoc.rs +++ b/services/src/pro/api/apidoc.rs @@ -54,6 +54,7 @@ use crate::layers::listing::{ }; use crate::pro; use crate::pro::api::handlers::users::{Quota, UpdateQuota}; +use crate::pro::datasets::DatasetDeletionType; use crate::pro::datasets::Expiration; use crate::pro::datasets::ExpirationChange; use crate::pro::permissions::{ @@ -376,6 +377,7 @@ use utoipa::{Modify, OpenApi}; DataPath, Expiration, ExpirationChange, + DatasetDeletionType, PlotOutputFormat, WrappedPlotOutput, diff --git a/services/src/pro/api/handlers/datasets.rs b/services/src/pro/api/handlers/datasets.rs index b3e41fc1f..13a7a101d 100644 --- a/services/src/pro/api/handlers/datasets.rs +++ b/services/src/pro/api/handlers/datasets.rs @@ -193,14 +193,12 @@ where request_body(content = ExpirationChange, examples( ("Delete" = (value = json!({ "type": "setExpire", - "delete_record": true, - "delete_data": true + "deletionType": "DeleteData" }))), ("Expire" = (value = json!({ "type": "setExpire", - "delete_record": true, - "delete_data": true, - "deletion_timestamp": "2024-06-28T14:52:39.655Z" + "deletionType": "DeleteRecordAndData", + "deletionTimestamp": "2024-06-28T14:52:39.655Z" }))), ("Undo Expire" = (value = json!({ "type": "unsetExpire" diff --git a/services/src/pro/datasets/mod.rs b/services/src/pro/datasets/mod.rs index c96e11de3..0f7c4ecdc 100644 --- a/services/src/pro/datasets/mod.rs +++ b/services/src/pro/datasets/mod.rs @@ -8,5 +8,6 @@ pub use external::{ }; pub use storage::{ - ChangeDatasetExpiration, Expiration, ExpirationChange, UploadedUserDatasetStore, + ChangeDatasetExpiration, DatasetDeletionType, Expiration, ExpirationChange, + UploadedUserDatasetStore, }; diff --git a/services/src/pro/datasets/storage.rs b/services/src/pro/datasets/storage.rs index c09c5dfdd..cbdb0e4a8 100644 --- a/services/src/pro/datasets/storage.rs +++ b/services/src/pro/datasets/storage.rs @@ -14,6 +14,7 @@ use crate::error::Result; use crate::pro::datasets::storage::DatasetDeletionType::{DeleteData, DeleteRecordAndData}; #[derive(Deserialize, Serialize, Debug, Clone, ToSchema)] +#[serde(rename_all = "camelCase")] pub struct Expiration { pub deletion_timestamp: Option, pub deletion_type: DatasetDeletionType, From c287c3459b425802b5a54c2060145cbb3a955115 Mon Sep 17 00:00:00 2001 From: Nikolaus Glombiewski Date: Wed, 10 Jul 2024 16:38:17 +0200 Subject: [PATCH 06/17] Add reserved tags; --- Cargo.lock | 1 + services/Cargo.toml | 1 + services/src/api/handlers/datasets.rs | 4 ++- services/src/datasets/storage.rs | 25 +++++++++++++ services/src/pro/api/handlers/datasets.rs | 4 ++- services/src/pro/datasets/postgres.rs | 44 ++++++++++++++--------- 6 files changed, 61 insertions(+), 18 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1a6f21221..4bcfd4ff6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2883,6 +2883,7 @@ dependencies = [ "snafu", "stream-cancel", "strum", + "strum_macros", "tempfile", "time", "tokio", diff --git a/services/Cargo.toml b/services/Cargo.toml index 7ebd3eed6..b9fc25f26 100644 --- a/services/Cargo.toml +++ b/services/Cargo.toml @@ -120,6 +120,7 @@ xgboost-rs = { version = "0.3", optional = true, features = ["use_serde"] } zip = "0.6" assert-json-diff = "2.0.2" sha2 = "0.10.8" +strum_macros = "0.26.1" [target.'cfg(target_os = "linux")'.dependencies] nix = { version = "0.27", features = ["socket"] } diff --git a/services/src/api/handlers/datasets.rs b/services/src/api/handlers/datasets.rs index 1f83ada89..a192a6697 100755 --- a/services/src/api/handlers/datasets.rs +++ b/services/src/api/handlers/datasets.rs @@ -1,3 +1,4 @@ +use crate::datasets::storage::{check_reserved_tags, ReservedTags}; use crate::{ api::model::{ responses::datasets::{errors::*, DatasetNameResponse}, @@ -579,7 +580,8 @@ pub async fn create_upload_dataset( let db = app_ctx.session_context(session).db(); let upload = db.load_upload(upload_id).await.context(UploadNotFound)?; - add_tag(&mut definition.properties, "upload".to_owned()); + check_reserved_tags(&definition.properties.tags); + add_tag(&mut definition.properties, ReservedTags::Upload.to_string()); adjust_meta_data_path(&mut definition.meta_data, &upload) .context(CannotResolveUploadFilePath)?; diff --git a/services/src/datasets/storage.rs b/services/src/datasets/storage.rs index db0f3d037..ccb148c97 100755 --- a/services/src/datasets/storage.rs +++ b/services/src/datasets/storage.rs @@ -19,6 +19,10 @@ use geoengine_operators::{mock::MockDatasetDataSourceLoadingInfo, source::GdalMe use serde::{Deserialize, Serialize}; use snafu::ResultExt; use std::fmt::Debug; +use std::str::FromStr; +use serde_json::json; +use strum_macros; +use strum_macros::{Display, EnumString}; use utoipa::{IntoParams, ToSchema}; use uuid::Uuid; use validator::{Validate, ValidationError}; @@ -95,6 +99,13 @@ pub struct AutoCreateDataset { pub tags: Option>, } +#[derive(Display, EnumString)] +pub enum ReservedTags { + #[strum(serialize = "upload")] + Upload, + Deleted, +} + fn validate_main_file(main_file: &str) -> Result<(), ValidationError> { if main_file.is_empty() || main_file.contains('/') || main_file.contains("..") { return Err(ValidationError::new("Invalid upload file name")); @@ -114,6 +125,20 @@ pub fn validate_tags(tags: &Vec) -> Result<(), ValidationError> { Ok(()) } +pub fn check_reserved_tags(tags: &Option>) { + if let Some(tags) = tags { + for tag in tags { + let conversion = ReservedTags::from_str(tag.as_str()); + if let Ok(reserved) = conversion { + log::warn!( + "Adding a new dataset with a reserved tag: {}", + reserved.to_string() + ); + } + } + } +} + #[derive(Deserialize, Serialize, Debug, Clone, IntoParams)] #[serde(rename_all = "camelCase")] pub struct SuggestMetaData { diff --git a/services/src/pro/api/handlers/datasets.rs b/services/src/pro/api/handlers/datasets.rs index 13a7a101d..f64f29693 100644 --- a/services/src/pro/api/handlers/datasets.rs +++ b/services/src/pro/api/handlers/datasets.rs @@ -1,5 +1,6 @@ use crate::api::handlers::datasets::add_tag; use crate::datasets::listing::DatasetProvider; +use crate::datasets::storage::{check_reserved_tags, ReservedTags}; use crate::datasets::upload::{UploadDb, UploadId}; use crate::datasets::DatasetName; use crate::pro::datasets::{ChangeDatasetExpiration, ExpirationChange, UploadedUserDatasetStore}; @@ -167,7 +168,8 @@ where let db = app_ctx.session_context(session).db(); let upload = db.load_upload(upload_id).await.context(UploadNotFound)?; - add_tag(&mut definition.properties, "upload".to_owned()); + check_reserved_tags(&definition.properties.tags); + add_tag(&mut definition.properties, ReservedTags::Upload.to_string()); adjust_meta_data_path(&mut definition.meta_data, &upload) .context(CannotResolveUploadFilePath)?; diff --git a/services/src/pro/datasets/postgres.rs b/services/src/pro/datasets/postgres.rs index 22b2300da..a09b14075 100644 --- a/services/src/pro/datasets/postgres.rs +++ b/services/src/pro/datasets/postgres.rs @@ -3,7 +3,9 @@ use crate::datasets::listing::Provenance; use crate::datasets::listing::{DatasetListOptions, DatasetListing, DatasetProvider}; use crate::datasets::listing::{OrderBy, ProvenanceOutput}; use crate::datasets::postgres::resolve_dataset_name_to_id; -use crate::datasets::storage::{Dataset, DatasetDb, DatasetStore, MetaDataDefinition}; +use crate::datasets::storage::{ + Dataset, DatasetDb, DatasetStore, MetaDataDefinition, ReservedTags, +}; use crate::datasets::upload::{delete_upload, FileId}; use crate::datasets::upload::{Upload, UploadDb, UploadId}; use crate::datasets::{AddDataset, DatasetIdAndName, DatasetName}; @@ -82,7 +84,10 @@ where pos += 1; (format!("AND d.tags @> ${pos}::text[]"), filter_tags.clone()) } else { - ("AND NOT d.tags @> '{deleted}'::text[]".to_string(), vec![]) + ( + format!("AND NOT d.tags @> '{{{}}}'::text[]", ReservedTags::Deleted), + vec![], + ) }; let stmt = conn @@ -1065,11 +1070,12 @@ where let tag_deletion = tx .prepare( - " + format!( + " UPDATE datasets SET - tags = tags || '{deleted}' + tags = tags || '{{{}}}' FROM user_permitted_datasets p, uploaded_user_datasets u WHERE @@ -1077,6 +1083,9 @@ where AND p.dataset_id = datasets.id AND u.dataset_id = datasets.id AND u.status = $3 AND u.expiration <= CURRENT_TIMESTAMP AND u.deletion_type = $4;", + ReservedTags::Deleted + ) + .as_str(), ) .await?; newly_expired_datasets += tx @@ -1144,17 +1153,21 @@ where let tag_deletion = tx .prepare( - " + format!( + " UPDATE datasets SET - tags = tags || '{deleted}' + tags = tags || '{{{}}}' FROM user_permitted_datasets p, uploaded_user_datasets u WHERE p.user_id = $1 AND p.dataset_id = datasets.id AND u.dataset_id = datasets.id AND u.status = $2 AND u.expiration <= CURRENT_TIMESTAMP AND u.deletion_type = $3;", + ReservedTags::Deleted + ) + .as_str(), ) .await?; tx.execute( @@ -1207,17 +1220,21 @@ where let tag_deletion = tx .prepare( - " + format!( + " UPDATE datasets SET - tags = tags || '{deleted}' + tags = tags || '{{{}}}' FROM uploaded_user_datasets u WHERE u.dataset_id = datasets.id AND u.status = $1 AND u.expiration <= CURRENT_TIMESTAMP AND u.deletion_type = $2;", + ReservedTags::Deleted + ) + .as_str(), ) .await?; tx.execute(&tag_deletion, &[&Expires, &DeleteData]).await?; @@ -1373,12 +1390,6 @@ where dataset.tags ); - if let Some(tags) = &dataset.tags { - if tags.contains(&"deleted".to_string()) { - log::warn!("Adding a new dataset with a deleted tag"); - } - } - self.check_namespace(&name)?; let typed_meta_data = meta_data.to_typed_metadata(); @@ -1900,14 +1911,15 @@ mod tests { } fn listing_not_deleted(dataset: &DatasetListing, origin: &UploadedTestDataset) -> bool { - dataset.name == origin.dataset_name && !dataset.tags.contains(&"deleted".to_owned()) + dataset.name == origin.dataset_name + && !dataset.tags.contains(&ReservedTags::Deleted.to_string()) } fn dataset_deleted(dataset: &Dataset, origin: &UploadedTestDataset) -> bool { let tags = dataset.tags.clone().unwrap(); let mut num_deleted = 0; for tag in tags { - if tag == *"deleted" { + if tag == ReservedTags::Deleted.to_string() { num_deleted += 1; } } From 2865e61058c4a931cf41c73e497f2e812bc78b17 Mon Sep 17 00:00:00 2001 From: Nikolaus Glombiewski Date: Wed, 10 Jul 2024 17:31:03 +0200 Subject: [PATCH 07/17] Reuse connections and wrap dataset status updates; --- services/src/datasets/storage.rs | 1 - services/src/pro/datasets/postgres.rs | 70 ++++++++++++++------------- services/src/pro/datasets/storage.rs | 15 ++++-- 3 files changed, 47 insertions(+), 39 deletions(-) diff --git a/services/src/datasets/storage.rs b/services/src/datasets/storage.rs index ccb148c97..ac3d73fb4 100755 --- a/services/src/datasets/storage.rs +++ b/services/src/datasets/storage.rs @@ -20,7 +20,6 @@ use serde::{Deserialize, Serialize}; use snafu::ResultExt; use std::fmt::Debug; use std::str::FromStr; -use serde_json::json; use strum_macros; use strum_macros::{Display, EnumString}; use utoipa::{IntoParams, ToSchema}; diff --git a/services/src/pro/datasets/postgres.rs b/services/src/pro/datasets/postgres.rs index a09b14075..44437bae1 100644 --- a/services/src/pro/datasets/postgres.rs +++ b/services/src/pro/datasets/postgres.rs @@ -26,8 +26,10 @@ use crate::pro::permissions::{Permission, RoleId}; use crate::projects::Symbology; use crate::util::postgres::PostgresErrorExt; use async_trait::async_trait; +use bb8_postgres::bb8::PooledConnection; use bb8_postgres::tokio_postgres::tls::{MakeTlsConnect, TlsConnect}; use bb8_postgres::tokio_postgres::Socket; +use bb8_postgres::PostgresConnectionManager; use geoengine_datatypes::dataset::{DataId, DatasetId}; use geoengine_datatypes::error::BoxedResultExt; use geoengine_datatypes::primitives::RasterQueryRectangle; @@ -63,8 +65,8 @@ where <>::TlsConnect as TlsConnect>::Future: Send, { async fn list_datasets(&self, options: DatasetListOptions) -> Result> { - self.update_datasets_status().await?; - let conn = self.conn_pool.get().await?; + let mut conn = self.conn_pool.get().await?; + self.lazy_dataset_store_updates(&mut conn, None).await?; let mut pos = 3; let order_sql = if options.order == OrderBy::NameAsc { @@ -186,8 +188,10 @@ where } async fn load_dataset(&self, dataset: &DatasetId) -> Result { - self.update_dataset_status(dataset).await?; - let conn = self.conn_pool.get().await?; + let mut conn = self.conn_pool.get().await?; + self.lazy_dataset_store_updates(&mut conn, Some(dataset)) + .await?; + let stmt = conn .prepare( " @@ -231,8 +235,9 @@ where } async fn load_provenance(&self, dataset: &DatasetId) -> Result { - self.update_dataset_status(dataset).await?; - let conn = self.conn_pool.get().await?; + let mut conn = self.conn_pool.get().await?; + self.lazy_dataset_store_updates(&mut conn, Some(dataset)) + .await?; let stmt = conn .prepare( @@ -260,8 +265,9 @@ where } async fn load_loading_info(&self, dataset: &DatasetId) -> Result { - self.update_dataset_status(dataset).await?; - let conn = self.conn_pool.get().await?; + let mut conn = self.conn_pool.get().await?; + self.lazy_dataset_store_updates(&mut conn, Some(dataset)) + .await?; let stmt = conn .prepare( @@ -287,8 +293,8 @@ where &self, dataset_name: &DatasetName, ) -> Result> { - self.update_datasets_status().await?; - let conn = self.conn_pool.get().await?; + let mut conn = self.conn_pool.get().await?; + self.lazy_dataset_store_updates(&mut conn, None).await?; resolve_dataset_name_to_id(&conn, dataset_name).await } @@ -299,8 +305,8 @@ where limit: u32, offset: u32, ) -> Result> { - self.update_datasets_status().await?; - let connection = self.conn_pool.get().await?; + let mut conn = self.conn_pool.get().await?; + self.lazy_dataset_store_updates(&mut conn, None).await?; let limit = i64::from(limit); let offset = i64::from(offset); @@ -319,7 +325,7 @@ where String::new() }; - let stmt = connection + let stmt = conn .prepare(&format!( " SELECT @@ -336,7 +342,7 @@ where )) .await?; - let rows = connection.query(&stmt, &query_params).await?; + let rows = conn.query(&stmt, &query_params).await?; Ok(rows.iter().map(|row| row.get(0)).collect()) } @@ -914,7 +920,7 @@ impl From for crate::datasets::upload::FileUpload { } #[async_trait] -impl TxUploadedUserDatasetStore for ProPostgresDb +impl TxUploadedUserDatasetStore> for ProPostgresDb where Tls: MakeTlsConnect + Clone + Send + Sync + 'static + std::fmt::Debug, >::Stream: Send + Sync, @@ -1034,6 +1040,22 @@ where Ok(internal_status.into()) } + async fn lazy_dataset_store_updates( + &self, + conn: &mut PooledConnection>, + dataset_id: Option<&DatasetId>, + ) -> Result<()> { + let tx = conn.build_transaction().start().await?; + if let Some(id) = dataset_id { + self.update_dataset_status_in_tx(&tx, id).await?; + } else { + self.update_datasets_status_in_tx(&tx).await?; + } + tx.commit().await?; + + Ok(()) + } + async fn update_dataset_status_in_tx( &self, tx: &Transaction, @@ -1504,24 +1526,6 @@ where Ok(()) } - async fn update_dataset_status(&self, dataset_id: &DatasetId) -> Result<()> { - let mut conn = self.conn_pool.get().await?; - let tx = conn.build_transaction().start().await?; - self.update_dataset_status_in_tx(&tx, dataset_id).await?; - tx.commit().await?; - - Ok(()) - } - - async fn update_datasets_status(&self) -> Result<()> { - let mut conn = self.conn_pool.get().await?; - let tx = conn.build_transaction().start().await?; - self.update_datasets_status_in_tx(&tx).await?; - tx.commit().await?; - - Ok(()) - } - async fn clear_expired_datasets(&self) -> Result { ensure!(self.session.is_admin(), error::PermissionDenied); diff --git a/services/src/pro/datasets/storage.rs b/services/src/pro/datasets/storage.rs index cbdb0e4a8..75d4d49d6 100644 --- a/services/src/pro/datasets/storage.rs +++ b/services/src/pro/datasets/storage.rs @@ -1,4 +1,5 @@ use async_trait::async_trait; +use bb8_postgres::bb8::{ManageConnection, PooledConnection}; use postgres_types::{FromSql, ToSql}; use serde::{Deserialize, Serialize}; use tokio_postgres::Transaction; @@ -122,7 +123,7 @@ impl From for UploadedDatasetStatus { /// In contrast to the `UploadedUserDatasetStore` this is not to be used by services but only by the `ProPostgresDb` internally. /// This is because services do not know about database transactions. #[async_trait] -pub trait TxUploadedUserDatasetStore { +pub trait TxUploadedUserDatasetStore { async fn validate_expiration_request_in_tx( &self, tx: &Transaction, @@ -136,6 +137,14 @@ pub trait TxUploadedUserDatasetStore { tx: &Transaction, ) -> Result; + /// Updates the status of datasets, because some datasets might have reached expiration + //TODO: Add some sort of periodic update for the status of datasets + async fn lazy_dataset_store_updates( + &self, + conn: &mut PooledConnection, + dataset_id: Option<&DatasetId>, + ) -> Result<()>; + async fn update_dataset_status_in_tx( &self, tx: &Transaction, @@ -170,9 +179,5 @@ pub trait UploadedUserDatasetStore { async fn expire_uploaded_dataset(&self, expire_dataset: ChangeDatasetExpiration) -> Result<()>; - async fn update_dataset_status(&self, dataset_id: &DatasetId) -> Result<()>; - - async fn update_datasets_status(&self) -> Result<()>; - async fn clear_expired_datasets(&self) -> Result; } From 265eba09b4725423b2064fad2ad936bbf66f69c0 Mon Sep 17 00:00:00 2001 From: Nikolaus Glombiewski Date: Wed, 10 Jul 2024 18:03:10 +0200 Subject: [PATCH 08/17] Add display errors messages for expiration errors; --- operators/src/error.rs | 5 +++-- services/src/error.rs | 9 ++++++++- services/src/pro/datasets/postgres.rs | 18 ++++++++++-------- 3 files changed, 21 insertions(+), 11 deletions(-) diff --git a/operators/src/error.rs b/operators/src/error.rs index 7356e8094..41ea26f14 100644 --- a/operators/src/error.rs +++ b/operators/src/error.rs @@ -1,6 +1,6 @@ use crate::util::statistics::StatisticsError; use bb8_postgres::bb8; -use geoengine_datatypes::dataset::{DataId, NamedData}; +use geoengine_datatypes::dataset::{DataId, DatasetId, NamedData}; use geoengine_datatypes::error::ErrorSource; use geoengine_datatypes::primitives::{FeatureDataType, TimeInterval}; use geoengine_datatypes::raster::RasterDataType; @@ -501,8 +501,9 @@ pub enum Error { source: bb8::RunError, }, + #[snafu(display("Dataset {} cannot be accessed, because it was deleted", id))] DatasetDeleted { - id: String, + id: DatasetId, }, } diff --git a/services/src/error.rs b/services/src/error.rs index ff93cb2fa..6a1b90b48 100644 --- a/services/src/error.rs +++ b/services/src/error.rs @@ -492,11 +492,18 @@ pub enum Error { resource_id: String, }, - ExpirationTimestampInPast, + #[snafu(display("Trying to set an expiration timestamp for Dataset {dataset} in the past"))] + ExpirationTimestampInPast { + dataset: DatasetId, + }, + #[snafu(display("Illegal expiration update for Dataset {dataset}: {reason}"))] IllegalExpirationUpdate { + dataset: DatasetId, reason: String, }, + #[snafu(display("Illegal status for Dataset {dataset}: {status}"))] IllegalDatasetStatus { + dataset: DatasetId, status: String, }, } diff --git a/services/src/pro/datasets/postgres.rs b/services/src/pro/datasets/postgres.rs index 44437bae1..49a08f71b 100644 --- a/services/src/pro/datasets/postgres.rs +++ b/services/src/pro/datasets/postgres.rs @@ -417,9 +417,7 @@ where let uploaded_status = self.uploaded_dataset_status_in_tx(&id, &tx).await; if let Ok(status) = uploaded_status { if matches!(status, UploadedDatasetStatus::Deleted) { - return Err(geoengine_operators::error::Error::DatasetDeleted { - id: id.to_string(), - }); + return Err(geoengine_operators::error::Error::DatasetDeleted { id }); } } @@ -510,9 +508,7 @@ where let uploaded_status = self.uploaded_dataset_status_in_tx(&id, &tx).await; if let Ok(status) = uploaded_status { if matches!(status, UploadedDatasetStatus::Deleted) { - return Err(geoengine_operators::error::Error::DatasetDeleted { - id: id.to_string(), - }); + return Err(geoengine_operators::error::Error::DatasetDeleted { id }); } } @@ -980,7 +976,9 @@ where match status { Available | Expires => { if !legal_expiration { - return Err(ExpirationTimestampInPast); + return Err(ExpirationTimestampInPast { + dataset: (*dataset_id).into(), + }); } } Expired | UpdateExpired | Deleted => { @@ -988,17 +986,20 @@ where && matches!(deletion_type, Some(DeleteRecordAndData)) { return Err(IllegalExpirationUpdate { + dataset: (*dataset_id).into(), reason: "Prior expiration already deleted data and record".to_string(), }); } if expiration.deletion_timestamp.is_some() { return Err(IllegalExpirationUpdate { + dataset: (*dataset_id).into(), reason: "Setting expiration after deletion".to_string(), }); } } DeletedWithError => { return Err(IllegalDatasetStatus { + dataset: (*dataset_id).into(), status: "Dataset was deleted, but an error occurred during deletion" .to_string(), }); @@ -1378,6 +1379,7 @@ where .await?; if set_changes == 0 { return Err(IllegalDatasetStatus { + dataset: (*dataset_id).into(), status: "Requested dataset does not exist or does not have an expiration" .to_string(), }); @@ -2268,7 +2270,7 @@ mod tests { )) .await; assert!(err.is_err()); - assert!(matches!(err.unwrap_err(), ExpirationTimestampInPast)); + assert!(matches!(err.unwrap_err(), ExpirationTimestampInPast { .. })); //Unset expire for non-expiring dataset let err = db From 35eb211c5b6ef49308b22b3ada62e36c1dc2cc14 Mon Sep 17 00:00:00 2001 From: Nikolaus Glombiewski Date: Thu, 11 Jul 2024 18:14:07 +0200 Subject: [PATCH 09/17] Fix listing with empty tags; --- services/src/pro/datasets/postgres.rs | 46 ++++++++++++++++++++++++--- 1 file changed, 42 insertions(+), 4 deletions(-) diff --git a/services/src/pro/datasets/postgres.rs b/services/src/pro/datasets/postgres.rs index 49a08f71b..f0d0366d4 100644 --- a/services/src/pro/datasets/postgres.rs +++ b/services/src/pro/datasets/postgres.rs @@ -87,7 +87,10 @@ where (format!("AND d.tags @> ${pos}::text[]"), filter_tags.clone()) } else { ( - format!("AND NOT d.tags @> '{{{}}}'::text[]", ReservedTags::Deleted), + format!( + "AND (d.tags IS NULL OR NOT d.tags @> '{{{}}}'::text[])", + ReservedTags::Deleted + ), vec![], ) }; @@ -1791,7 +1794,7 @@ mod tests { upload_id: UploadId, } - fn test_point_dataset(name_space: String, name: &str) -> TestDatasetDefinition { + fn test_point_dataset(name_space: Option, name: &str) -> TestDatasetDefinition { let local_path = PathBuf::from(TEST_POINT_DATASET_SOURCE_PATH); let file_name = local_path.file_name().unwrap().to_str().unwrap(); let loading_info = OgrSourceDataset { @@ -1852,7 +1855,7 @@ mod tests { phantom: Default::default(), }); - let dataset_name = DatasetName::new(Some(name_space), name); + let dataset_name = DatasetName::new(name_space, name); TestDatasetDefinition { meta_data, @@ -1885,7 +1888,7 @@ mod tests { name: &str, upload_dir: &mut TestDataUploads, ) -> UploadedTestDataset { - let test_dataset = test_point_dataset(user_session.user.id.to_string(), name); + let test_dataset = test_point_dataset(Some(user_session.user.id.to_string()), name); let upload_id = upload_point_dataset(app_ctx, user_session.id).await; let res = app_ctx @@ -1956,6 +1959,41 @@ mod tests { .unwrap() } + #[ge_context::test] + async fn it_lists_datasets_without_tags(app_ctx: ProPostgresContext) { + let admin_session = admin_login(&app_ctx).await; + let admin_ctx = app_ctx.session_context(admin_session.clone()); + let db = admin_ctx.db(); + let test_dataset = test_point_dataset(None, "test_data"); + + let ds = AddDataset { + name: None, + display_name: "TestData".to_string(), + description: "TestData without tags".to_string(), + source_operator: "OgrSource".to_string(), + symbology: None, + provenance: None, + tags: None, + }; + + db.add_dataset(ds, test_dataset.meta_data).await.unwrap(); + + let default_list_options = DatasetListOptions { + filter: None, + order: OrderBy::NameAsc, + offset: 0, + limit: 10, + tags: None, + }; + + let listing = db + .list_datasets(default_list_options.clone()) + .await + .unwrap(); + + assert_eq!(listing.len(), 1); + } + #[ge_context::test] async fn it_deletes_datasets(app_ctx: ProPostgresContext) { let mut test_data = TestDataUploads::default(); From fd28d7b3bea399b0324b54586bd1d5a62b9653dd Mon Sep 17 00:00:00 2001 From: Nikolaus Glombiewski Date: Tue, 23 Jul 2024 16:05:21 +0200 Subject: [PATCH 10/17] Add dataset status request; Unify update methods and add view with updatable dataset entries; --- services/src/pro/api/apidoc.rs | 8 +- services/src/pro/api/handlers/datasets.rs | 80 ++- .../contexts/migrations/current_schema.sql | 12 + ...migration_0010_delete_uploaded_datasets.rs | 12 + services/src/pro/datasets/mod.rs | 4 +- services/src/pro/datasets/postgres.rs | 514 ++++++++++-------- services/src/pro/datasets/storage.rs | 112 +++- .../pro/permissions/postgres_permissiondb.rs | 48 +- 8 files changed, 544 insertions(+), 246 deletions(-) diff --git a/services/src/pro/api/apidoc.rs b/services/src/pro/api/apidoc.rs index 23d735e84..e8aff5c58 100644 --- a/services/src/pro/api/apidoc.rs +++ b/services/src/pro/api/apidoc.rs @@ -54,9 +54,9 @@ use crate::layers::listing::{ }; use crate::pro; use crate::pro::api::handlers::users::{Quota, UpdateQuota}; -use crate::pro::datasets::DatasetDeletionType; -use crate::pro::datasets::Expiration; -use crate::pro::datasets::ExpirationChange; +use crate::pro::datasets::{ + DatasetAccessStatusResponse, DatasetDeletionType, Expiration, ExpirationChange, +}; use crate::pro::permissions::{ Permission, PermissionListing, ResourceId, Role, RoleDescription, RoleId, }; @@ -162,6 +162,7 @@ use utoipa::{Modify, OpenApi}; pro::api::handlers::permissions::remove_permission_handler, pro::api::handlers::permissions::get_resource_permissions_handler, pro::api::handlers::datasets::set_dataset_expiration, + pro::api::handlers::datasets::get_dataset_status, pro::api::handlers::datasets::gc_expired_datasets ), components( @@ -378,6 +379,7 @@ use utoipa::{Modify, OpenApi}; Expiration, ExpirationChange, DatasetDeletionType, + DatasetAccessStatusResponse, PlotOutputFormat, WrappedPlotOutput, diff --git a/services/src/pro/api/handlers/datasets.rs b/services/src/pro/api/handlers/datasets.rs index f64f29693..1f212ce75 100644 --- a/services/src/pro/api/handlers/datasets.rs +++ b/services/src/pro/api/handlers/datasets.rs @@ -3,7 +3,10 @@ use crate::datasets::listing::DatasetProvider; use crate::datasets::storage::{check_reserved_tags, ReservedTags}; use crate::datasets::upload::{UploadDb, UploadId}; use crate::datasets::DatasetName; -use crate::pro::datasets::{ChangeDatasetExpiration, ExpirationChange, UploadedUserDatasetStore}; +use crate::pro::datasets::{ + ChangeDatasetExpiration, DatasetAccessStatusResponse, ExpirationChange, + UploadedUserDatasetStore, +}; use crate::{ api::{ handlers::datasets::{ @@ -62,6 +65,9 @@ where web::resource("/{dataset}/expiration") .route(web::put().to(set_dataset_expiration::)), ) + .service( + web::resource("/{dataset}/status").route(web::get().to(get_dataset_status::)), + ) .service( web::resource("/{dataset}") .route(web::get().to(get_dataset_handler::)) @@ -284,12 +290,54 @@ where Ok(HttpResponse::Ok()) } +/// Returns the access status of the current user for the dataset with the given name. +#[utoipa::path( + tag = "Datasets", + get, + path = "/dataset/{dataset}/status", + responses( + (status = 200, description = "OK", body = DatasetAccessStatusResponse), + (status = 400, description = "Bad request", body = ErrorResponse), + (status = 401, response = crate::api::model::responses::UnauthorizedUserResponse) + ), + params( + ("dataset" = DatasetName, description = "Dataset Name"), + ), + security( + ("session_token" = []) + ) +)] +async fn get_dataset_status( + session: C::Session, + app_ctx: web::Data, + dataset: web::Path, +) -> Result +where + <::SessionContext as SessionContext>::GeoEngineDB: ProGeoEngineDb, +{ + let db = app_ctx.session_context(session).db(); + + let dataset_name = dataset.into_inner(); + + let dataset_id = db.resolve_dataset_name_to_id(&dataset_name).await?; + + let dataset_id = dataset_id.ok_or(error::Error::UnknownDatasetName { + dataset_name: dataset_name.to_string(), + })?; + + let status: DatasetAccessStatusResponse = + db.get_dataset_access_status(&dataset_id).await?.into(); + + Ok(web::Json(status)) +} + #[cfg(test)] mod tests { use super::*; use crate::api::model::responses::IdResponse; use crate::datasets::DatasetName; use crate::pro::contexts::ProPostgresContext; + use crate::pro::datasets::DatasetAccessStatusResponse; use crate::pro::ge_context; use crate::pro::util::tests::get_db_timestamp; use crate::{ @@ -685,7 +733,9 @@ mod tests { } #[ge_context::test] - async fn it_expires_dataset(app_ctx: ProPostgresContext) -> Result<()> { + async fn it_expires_dataset_and_checks_status( + app_ctx: ProPostgresContext, + ) -> Result<()> { let mut test_data = TestDataUploads::default(); let session = app_ctx.create_anonymous_session().await.unwrap(); @@ -707,6 +757,16 @@ mod tests { assert!(db.load_dataset(&dataset_id).await.is_ok()); + let req = actix_web::test::TestRequest::get() + .uri(&format!("/dataset/{dataset_name}/status")) + .append_header((header::AUTHORIZATION, Bearer::new(session_id.to_string()))); + let res = send_pro_test_request(req, app_ctx.clone()).await; + assert_eq!(res.status(), 200, "response: {res:?}"); + let status: DatasetAccessStatusResponse = actix_web::test::read_body_json(res).await; + assert!(status.is_user_upload); + assert!(status.is_available); + assert!(status.expiration.is_none()); + let current_time = get_db_timestamp(&app_ctx).await; let future_time = current_time.add(Duration::seconds(2)); @@ -726,10 +786,26 @@ mod tests { assert!(db.load_dataset(&dataset_id).await.is_ok()); + let req = actix_web::test::TestRequest::get() + .uri(&format!("/dataset/{dataset_name}/status")) + .append_header((header::AUTHORIZATION, Bearer::new(session_id.to_string()))); + let res = send_pro_test_request(req, app_ctx.clone()).await; + assert_eq!(res.status(), 200, "response: {res:?}"); + let status: DatasetAccessStatusResponse = actix_web::test::read_body_json(res).await; + assert!(status.is_user_upload); + assert!(status.is_available); + assert!(status.expiration.is_some()); + tokio::time::sleep(std::time::Duration::from_secs(2)).await; assert!(db.load_dataset(&dataset_id).await.is_err()); + let req = actix_web::test::TestRequest::get() + .uri(&format!("/dataset/{dataset_name}/status")) + .append_header((header::AUTHORIZATION, Bearer::new(session_id.to_string()))); + let res = send_pro_test_request(req, app_ctx.clone()).await; + assert_eq!(res.status(), 400, "response: {res:?}"); + Ok(()) } } diff --git a/services/src/pro/contexts/migrations/current_schema.sql b/services/src/pro/contexts/migrations/current_schema.sql index a9bfed7a0..a78ff9d58 100644 --- a/services/src/pro/contexts/migrations/current_schema.sql +++ b/services/src/pro/contexts/migrations/current_schema.sql @@ -253,3 +253,15 @@ CREATE TABLE uploaded_user_datasets ( deletion_type "DatasetDeletionType", PRIMARY KEY (user_id, dataset_id, upload_id) ); + +CREATE VIEW updatable_uploaded_user_datasets AS +SELECT + u.dataset_id, + u.user_id, + u.status, + u.deletion_type +FROM + uploaded_user_datasets u JOIN + user_permitted_datasets p ON (p.user_id = u.user_id) +WHERE + u.expiration <= CURRENT_TIMESTAMP AND (u.status = 'Expires' OR u.status = 'UpdateExpired'); diff --git a/services/src/pro/contexts/migrations/migration_0010_delete_uploaded_datasets.rs b/services/src/pro/contexts/migrations/migration_0010_delete_uploaded_datasets.rs index a166aa5cf..be4b02363 100644 --- a/services/src/pro/contexts/migrations/migration_0010_delete_uploaded_datasets.rs +++ b/services/src/pro/contexts/migrations/migration_0010_delete_uploaded_datasets.rs @@ -34,6 +34,18 @@ impl ProMigration for ProMigrationImpl { deletion_type "DatasetDeletionType", PRIMARY KEY (user_id, dataset_id, upload_id) ); + + CREATE VIEW updatable_uploaded_user_datasets AS + SELECT + u.dataset_id, + u.user_id, + u.status, + u.deletion_type + FROM + uploaded_user_datasets u JOIN + user_permitted_datasets p ON (u.user_id = p.user_id) + WHERE + u.expiration <= CURRENT_TIMESTAMP AND (u.status = 'Expires' OR u.status = 'UpdateExpired'); "#, ) .await?; diff --git a/services/src/pro/datasets/mod.rs b/services/src/pro/datasets/mod.rs index 0f7c4ecdc..d1a5f01f1 100644 --- a/services/src/pro/datasets/mod.rs +++ b/services/src/pro/datasets/mod.rs @@ -8,6 +8,6 @@ pub use external::{ }; pub use storage::{ - ChangeDatasetExpiration, DatasetDeletionType, Expiration, ExpirationChange, - UploadedUserDatasetStore, + ChangeDatasetExpiration, DatasetAccessStatus, DatasetAccessStatusResponse, DatasetDeletionType, + Expiration, ExpirationChange, UploadedUserDatasetStore, }; diff --git a/services/src/pro/datasets/postgres.rs b/services/src/pro/datasets/postgres.rs index f0d0366d4..cf4044263 100644 --- a/services/src/pro/datasets/postgres.rs +++ b/services/src/pro/datasets/postgres.rs @@ -17,12 +17,14 @@ use crate::pro::contexts::ProPostgresDb; use crate::pro::datasets::storage::DatasetDeletionType::{DeleteData, DeleteRecordAndData}; use crate::pro::datasets::storage::InternalUploadedDatasetStatus::{Deleted, DeletedWithError}; use crate::pro::datasets::storage::{ - ChangeDatasetExpiration, DatasetDeletionType, InternalUploadedDatasetStatus, - TxUploadedUserDatasetStore, UploadedDatasetStatus, UploadedUserDatasetStore, + ChangeDatasetExpiration, DatasetAccessStatus, DatasetDeletionType, DatasetType, + InternalUploadedDatasetStatus, TxUploadedUserDatasetStore, UploadedDatasetStatus, + UploadedUserDatasetStore, }; use crate::pro::datasets::{Expiration, ExpirationChange}; use crate::pro::permissions::postgres_permissiondb::TxPermissionDb; use crate::pro::permissions::{Permission, RoleId}; +use crate::pro::users::{UserId, UserSession}; use crate::projects::Symbology; use crate::util::postgres::PostgresErrorExt; use async_trait::async_trait; @@ -419,7 +421,7 @@ where let uploaded_status = self.uploaded_dataset_status_in_tx(&id, &tx).await; if let Ok(status) = uploaded_status { - if matches!(status, UploadedDatasetStatus::Deleted) { + if matches!(status, UploadedDatasetStatus::Deleted { .. }) { return Err(geoengine_operators::error::Error::DatasetDeleted { id }); } } @@ -510,7 +512,7 @@ where let uploaded_status = self.uploaded_dataset_status_in_tx(&id, &tx).await; if let Ok(status) = uploaded_status { - if matches!(status, UploadedDatasetStatus::Deleted) { + if matches!(status, UploadedDatasetStatus::Deleted { .. }) { return Err(geoengine_operators::error::Error::DatasetDeleted { id }); } } @@ -926,6 +928,59 @@ where >::TlsConnect: Send, <>::TlsConnect as TlsConnect>::Future: Send, { + async fn get_dataset_access_status_in_tx( + &self, + dataset_id: &DatasetId, + tx: &Transaction, + ) -> Result { + let permissions = self + .get_user_permissions_in_tx(*dataset_id, tx) + .await + .boxed_context(crate::error::PermissionDb)?; + let uploaded = self.uploaded_dataset_status_in_tx(dataset_id, tx).await; + let access_status = if let Ok(user_upload) = uploaded { + if let UploadedDatasetStatus::Deleted(expiration) = &user_upload { + if matches!(expiration.deletion_type, DeleteRecordAndData) { + return Err(UnknownDatasetId); + } + } + DatasetAccessStatus { + id: *dataset_id, + dataset_type: DatasetType::UserUpload(user_upload), + permissions, + } + } else { + let stmt = tx + .prepare( + " + SELECT + TRUE + FROM + user_permitted_datasets p JOIN datasets d + ON (p.dataset_id = d.id) + WHERE + d.id = $1 AND p.user_id = $2;", + ) + .await?; + + let rows = tx + .query(&stmt, &[&dataset_id, &self.session.user.id]) + .await?; + + if rows.is_empty() { + return Err(UnknownDatasetId); + } + + DatasetAccessStatus { + id: *dataset_id, + dataset_type: DatasetType::NonUserUpload, + permissions, + } + }; + + Ok(access_status) + } + async fn validate_expiration_request_in_tx( &self, tx: &Transaction, @@ -1020,13 +1075,14 @@ where .await .boxed_context(crate::error::PermissionDb)?; - self.update_dataset_status_in_tx(tx, dataset_id).await?; + self.update_uploaded_datasets_status_in_tx(tx, Some(dataset_id)) + .await?; let stmt = tx .prepare( " SELECT - status + status, expiration, deletion_type FROM uploaded_user_datasets WHERE @@ -1040,8 +1096,16 @@ where .ok_or(error::Error::UnknownDatasetId)?; let internal_status: InternalUploadedDatasetStatus = result.get(0); + let expiration_timestamp = result.get(1); + let dataset_deletion_type = result.get(2); - Ok(internal_status.into()) + let status = internal_status.convert_to_uploaded_dataset_status( + dataset_id, + expiration_timestamp, + dataset_deletion_type, + )?; + + Ok(status) } async fn lazy_dataset_store_updates( @@ -1050,235 +1114,136 @@ where dataset_id: Option<&DatasetId>, ) -> Result<()> { let tx = conn.build_transaction().start().await?; - if let Some(id) = dataset_id { - self.update_dataset_status_in_tx(&tx, id).await?; - } else { - self.update_datasets_status_in_tx(&tx).await?; - } + self.update_uploaded_datasets_status_in_tx(&tx, dataset_id) + .await?; tx.commit().await?; Ok(()) } - async fn update_dataset_status_in_tx( + #[allow(clippy::too_many_lines)] + async fn update_uploaded_datasets_status_in_tx( &self, tx: &Transaction, - dataset_id: &DatasetId, + dataset_id: Option<&DatasetId>, ) -> Result<()> { - let mut newly_expired_datasets = 0; - - let delete_records = tx - .prepare( - " - DELETE FROM - datasets - USING - user_permitted_datasets p, uploaded_user_datasets u - WHERE - p.user_id = $1 AND datasets.id = $2 - AND p.dataset_id = datasets.id AND u.dataset_id = datasets.id - AND (u.status = $3 OR u.status = $4) AND u.expiration <= CURRENT_TIMESTAMP - AND u.deletion_type = $5;", - ) - .await?; - newly_expired_datasets += tx - .execute( - &delete_records, - &[ - &self.session.user.id, - &dataset_id, - &Expires, - &UpdateExpired, - &DeleteRecordAndData, - ], - ) - .await?; - - let tag_deletion = tx - .prepare( - format!( - " - UPDATE - datasets - SET - tags = tags || '{{{}}}' - FROM - user_permitted_datasets p, uploaded_user_datasets u - WHERE - p.user_id = $1 AND datasets.id = $2 - AND p.dataset_id = datasets.id AND u.dataset_id = datasets.id - AND u.status = $3 AND u.expiration <= CURRENT_TIMESTAMP - AND u.deletion_type = $4;", - ReservedTags::Deleted - ) - .as_str(), - ) - .await?; - newly_expired_datasets += tx - .execute( - &tag_deletion, - &[&self.session.user.id, &dataset_id, &Expires, &DeleteData], - ) - .await?; - - if newly_expired_datasets > 0 { - let mark_deletion = tx - .prepare( - " - UPDATE - uploaded_user_datasets - SET - status = $3 - FROM - user_permitted_datasets p - WHERE - p.user_id = $1 AND uploaded_user_datasets.dataset_id = $2 - AND uploaded_user_datasets.dataset_id = p.dataset_id - AND (status = $4 OR status = $5) AND expiration <= CURRENT_TIMESTAMP;", - ) - .await?; + fn create_filter( + session: &UserSession, + dataset_id: Option<&DatasetId>, + mut param_size: usize, + ) -> (String, Option, String, Option) { + let (user_filter, user_param) = if session.is_admin() { + (String::new(), None) + } else { + param_size += 1; + let filter = format!("AND up.user_id = ${param_size}").to_string(); + (filter, Some(session.user.id)) + }; + + let (dataset_filter, dataset_param) = if let Some(dataset_id) = dataset_id { + param_size += 1; + let filter = format!("AND up.dataset_id = ${param_size}").to_string(); + (filter, Some(*dataset_id)) + } else { + (String::new(), None) + }; - tx.execute( - &mark_deletion, - &[ - &self.session.user.id, - &dataset_id, - &Expired, - &Expires, - &UpdateExpired, - ], - ) - .await?; + (user_filter, user_param, dataset_filter, dataset_param) } - Ok(()) - } - - async fn update_datasets_status_in_tx(&self, tx: &Transaction) -> Result<()> { - if self.session.is_admin() { - self.admin_update_datasets_status_in_tx(tx).await?; - } else { - let delete_records = tx - .prepare( - " - DELETE FROM - datasets - USING - user_permitted_datasets p, uploaded_user_datasets u - WHERE - p.user_id = $1 AND p.dataset_id = datasets.id AND u.dataset_id = datasets.id - AND u.status = $2 AND u.expiration <= CURRENT_TIMESTAMP - AND u.deletion_type = $3;", - ) - .await?; - tx.execute( - &delete_records, - &[&self.session.user.id, &Expires, &DeleteRecordAndData], - ) - .await?; + fn create_filter_params<'a>( + filter_params: &'a mut Vec<&'a (dyn ToSql + Sync)>, + user_id: Option<&'a UserId>, + dataset_id: Option<&'a DatasetId>, + ) -> &'a [&'a (dyn ToSql + Sync)] { + if let Some(user_id) = user_id { + filter_params.push(user_id); + } + if let Some(dataset_id) = dataset_id { + filter_params.push(dataset_id); + } + filter_params.as_slice() + } - let tag_deletion = tx - .prepare( - format!( - " + let (user_filter, user_param, dataset_filter, dataset_param) = + create_filter(&self.session, dataset_id, 1); + let tag_deletion = tx + .prepare( + format!(" UPDATE datasets SET tags = tags || '{{{}}}' FROM - user_permitted_datasets p, uploaded_user_datasets u + updatable_uploaded_user_datasets up WHERE - p.user_id = $1 AND p.dataset_id = datasets.id AND u.dataset_id = datasets.id - AND u.status = $2 AND u.expiration <= CURRENT_TIMESTAMP - AND u.deletion_type = $3;", - ReservedTags::Deleted - ) - .as_str(), + datasets.id = up.dataset_id AND up.deletion_type = $1 {user_filter} {dataset_filter};", + ReservedTags::Deleted ) - .await?; - tx.execute( - &tag_deletion, - &[&self.session.user.id, &Expires, &DeleteData], + .as_str(), ) .await?; + let mut tag_deletion_params: Vec<&(dyn ToSql + Sync)> = vec![&DeleteData]; + tx.execute( + &tag_deletion, + create_filter_params( + &mut tag_deletion_params, + user_param.as_ref(), + dataset_param.as_ref(), + ), + ) + .await?; - let mark_deletion = tx - .prepare( - " + let mark_deletion = tx + .prepare( + format!(" UPDATE uploaded_user_datasets SET - status = $2 + status = $1 FROM - user_permitted_datasets p + updatable_uploaded_user_datasets up WHERE - p.user_id = $1 AND uploaded_user_datasets.dataset_id = p.dataset_id - AND (status = $3 OR status = $4) AND expiration <= CURRENT_TIMESTAMP;", - ) - .await?; - - tx.execute( - &mark_deletion, - &[&self.session.user.id, &Expired, &Expires, &UpdateExpired], - ) - .await?; - } - Ok(()) - } - - async fn admin_update_datasets_status_in_tx(&self, tx: &Transaction) -> Result<()> { - ensure!(self.session.is_admin(), error::PermissionDenied); - let delete_records = tx - .prepare( - " - DELETE FROM - datasets - USING - user_permitted_datasets p, uploaded_user_datasets u - WHERE - u.dataset_id = datasets.id - AND u.status = $1 AND u.expiration <= CURRENT_TIMESTAMP - AND u.deletion_type = $2;", - ) - .await?; - tx.execute(&delete_records, &[&Expires, &DeleteRecordAndData]) - .await?; - - let tag_deletion = tx - .prepare( - format!( - " - UPDATE - datasets - SET - tags = tags || '{{{}}}' - FROM - uploaded_user_datasets u - WHERE - u.dataset_id = datasets.id - AND u.status = $1 AND u.expiration <= CURRENT_TIMESTAMP - AND u.deletion_type = $2;", - ReservedTags::Deleted + uploaded_user_datasets.dataset_id = up.dataset_id {user_filter} {dataset_filter};" ) .as_str(), ) .await?; - tx.execute(&tag_deletion, &[&Expires, &DeleteData]).await?; + let mut mark_deletion_params: Vec<&(dyn ToSql + Sync)> = vec![&Expired]; + tx.execute( + &mark_deletion, + create_filter_params( + &mut mark_deletion_params, + user_param.as_ref(), + dataset_param.as_ref(), + ), + ) + .await?; - let mark_deletion = tx + let (user_filter, user_param, dataset_filter, dataset_param) = + create_filter(&self.session, dataset_id, 2); + let delete_records = tx .prepare( - " - UPDATE - uploaded_user_datasets - SET - status = $1 - WHERE - (status = $2 or status = $3) AND expiration <= CURRENT_TIMESTAMP;", + format!(" + DELETE FROM + datasets + USING + uploaded_user_datasets up + WHERE + datasets.id = up.dataset_id AND up.status = $1 AND up.deletion_type = $2 {user_filter} {dataset_filter};").as_str(), ) .await?; - - tx.execute(&mark_deletion, &[&Expired, &Expires, &UpdateExpired]) - .await?; + let mut delete_records_params: Vec<&(dyn ToSql + Sync)> = + vec![&Expired, &DeleteRecordAndData]; + tx.execute( + &delete_records, + create_filter_params( + &mut delete_records_params, + user_param.as_ref(), + dataset_param.as_ref(), + ), + ) + .await?; Ok(()) } @@ -1510,7 +1475,7 @@ where .await .boxed_context(error::PermissionDb)?; - self.update_dataset_status_in_tx(&tx, &expire_dataset.dataset_id) + self.update_uploaded_datasets_status_in_tx(&tx, Some(&expire_dataset.dataset_id)) .await?; match expire_dataset.expiration_change { @@ -1523,7 +1488,7 @@ where .await?; } } - self.update_dataset_status_in_tx(&tx, &expire_dataset.dataset_id) + self.update_uploaded_datasets_status_in_tx(&tx, Some(&expire_dataset.dataset_id)) .await?; tx.commit().await?; @@ -1531,13 +1496,31 @@ where Ok(()) } + async fn get_dataset_access_status( + &self, + dataset_id: &DatasetId, + ) -> Result { + let mut conn = self.conn_pool.get().await?; + self.lazy_dataset_store_updates(&mut conn, Some(dataset_id)) + .await?; + + let tx = conn.build_transaction().start().await?; + + let result = self.get_dataset_access_status_in_tx(dataset_id, &tx).await; + + tx.commit().await?; + + result + } + async fn clear_expired_datasets(&self) -> Result { ensure!(self.session.is_admin(), error::PermissionDenied); let mut conn = self.conn_pool.get().await?; let tx = conn.build_transaction().start().await?; - self.update_datasets_status_in_tx(&tx).await?; + self.update_uploaded_datasets_status_in_tx(&tx, None) + .await?; let update_expired = tx .prepare( @@ -1582,36 +1565,26 @@ where updated += 1; //Could hypothetically overflow } - if !deleted.is_empty() { - let mark_deletion = tx - .prepare( - " + let mark_deletion = tx + .prepare( + " UPDATE uploaded_user_datasets SET status = $1, deleted = CURRENT_TIMESTAMP WHERE status = $2 AND upload_id = ANY($3);", - ) - .await?; + ) + .await?; + + if !deleted.is_empty() { tx.execute(&mark_deletion, &[&Deleted, &Expired, &deleted]) .await?; } if !deleted_with_error.is_empty() { - let mark_error = tx - .prepare( - " - UPDATE - uploaded_user_datasets - SET - status = $1, deleted = CURRENT_TIMESTAMP - WHERE - status = $2 AND upload_id = ANY($3);", - ) - .await?; tx.execute( - &mark_error, + &mark_deletion, &[&DeletedWithError, &Expired, &deleted_with_error], ) .await?; @@ -1634,6 +1607,7 @@ mod tests { use crate::contexts::SessionId; use crate::datasets::upload::UploadRootPath; use crate::error::Error::PermissionDenied; + use crate::pro::permissions::{PermissionDb, Role}; use crate::pro::users::{UserCredentials, UserRegistration}; use crate::pro::util::tests::get_db_timestamp; use crate::pro::util::tests::{admin_login, send_pro_test_request}; @@ -1707,7 +1681,7 @@ mod tests { .is_empty()); } - async fn add_single_dataset(db: &ProPostgresDb, session: &UserSession) { + async fn add_single_dataset(db: &ProPostgresDb, session: &UserSession) -> DatasetName { let loading_info = OgrSourceDataset { file_name: PathBuf::from("test.csv"), layer_name: "test.csv".to_owned(), @@ -1779,6 +1753,8 @@ mod tests { ) .await .unwrap(); + + dataset_name } const TEST_POINT_DATASET_SOURCE_PATH: &str = "vector/data/points.fgb"; @@ -1919,6 +1895,32 @@ mod tests { } } + async fn add_test_volume_dataset(app_ctx: &ProPostgresContext) -> DatasetId { + let admin_session = admin_login(app_ctx).await; + let admin_ctx = app_ctx.session_context(admin_session.clone()); + let db = admin_ctx.db(); + let dataset_name = add_single_dataset(&db, &admin_session).await; + let dataset_id = db + .resolve_dataset_name_to_id(&dataset_name) + .await + .unwrap() + .unwrap(); + + db.add_permission( + Role::registered_user_role_id(), + dataset_id, + Permission::Read, + ) + .await + .unwrap(); + + db.add_permission(Role::anonymous_role_id(), dataset_id, Permission::Read) + .await + .unwrap(); + + dataset_id + } + fn listing_not_deleted(dataset: &DatasetListing, origin: &UploadedTestDataset) -> bool { dataset.name == origin.dataset_name && !dataset.tags.contains(&ReservedTags::Deleted.to_string()) @@ -1940,6 +1942,12 @@ mod tests { fs::read_dir(path).is_ok() } + fn has_read_and_owner_permissions(permissions: &[Permission]) { + assert_eq!(permissions.len(), 2); + assert!(permissions.contains(&Permission::Read)); + assert!(permissions.contains(&Permission::Owner)); + } + async fn register_test_user(app_ctx: &ProPostgresContext) -> UserSession { let _user_id = app_ctx .register_user(UserRegistration { @@ -2286,6 +2294,66 @@ mod tests { assert!(!dir_exists(&fair2full)); } + #[ge_context::test] + async fn it_handles_dataset_status(app_ctx: ProPostgresContext) { + let mut test_data = TestDataUploads::default(); + let current_time = get_db_timestamp(&app_ctx).await; + let future_time = current_time.add(Duration::seconds(3)); + + let volume_dataset = add_test_volume_dataset(&app_ctx).await; + + let user_session = register_test_user(&app_ctx).await; + let db = app_ctx.session_context(user_session.clone()).db(); + + let access_status = db.get_dataset_access_status(&volume_dataset).await.unwrap(); + assert!(matches!( + access_status.dataset_type, + DatasetType::NonUserUpload + )); + assert_eq!(access_status.permissions, vec![Permission::Read]); + + let user_dataset = + upload_and_add_point_dataset(&app_ctx, &user_session, "user_dataset", &mut test_data) + .await + .dataset_id; + + let access_status = db.get_dataset_access_status(&user_dataset).await.unwrap(); + assert!(matches!( + access_status.dataset_type, + DatasetType::UserUpload(UploadedDatasetStatus::Available) + )); + has_read_and_owner_permissions(&access_status.permissions); + + db.expire_uploaded_dataset(ChangeDatasetExpiration::expire_fair( + user_dataset, + future_time, + )) + .await + .unwrap(); + let access_status = db.get_dataset_access_status(&user_dataset).await.unwrap(); + assert!(matches!( + access_status.dataset_type, + DatasetType::UserUpload(UploadedDatasetStatus::Expires(ex)) if ex.deletion_timestamp.unwrap() == future_time + )); + has_read_and_owner_permissions(&access_status.permissions); + + tokio::time::sleep(std::time::Duration::from_secs(5)).await; + + let access_status = db.get_dataset_access_status(&user_dataset).await.unwrap(); + assert!(matches!( + access_status.dataset_type, + DatasetType::UserUpload(UploadedDatasetStatus::Deleted(ex)) if ex.deletion_timestamp.unwrap() == future_time + )); + has_read_and_owner_permissions(&access_status.permissions); + + db.expire_uploaded_dataset(ChangeDatasetExpiration::delete_full(user_dataset)) + .await + .unwrap(); + + let access_status = db.get_dataset_access_status(&user_dataset).await; + assert!(matches!(access_status, Err(UnknownDatasetId))); + } + #[ge_context::test] async fn it_handles_expiration_errors(app_ctx: ProPostgresContext) { let mut test_data = TestDataUploads::default(); diff --git a/services/src/pro/datasets/storage.rs b/services/src/pro/datasets/storage.rs index 75d4d49d6..bf8ddff67 100644 --- a/services/src/pro/datasets/storage.rs +++ b/services/src/pro/datasets/storage.rs @@ -11,8 +11,10 @@ use geoengine_datatypes::primitives::DateTime; use crate::datasets::storage::MetaDataDefinition; use crate::datasets::upload::UploadId; use crate::datasets::{AddDataset, DatasetIdAndName}; +use crate::error::Error::IllegalDatasetStatus; use crate::error::Result; use crate::pro::datasets::storage::DatasetDeletionType::{DeleteData, DeleteRecordAndData}; +use crate::pro::permissions::Permission; #[derive(Deserialize, Serialize, Debug, Clone, ToSchema)] #[serde(rename_all = "camelCase")] @@ -89,9 +91,11 @@ pub enum ExpirationChange { UnsetExpire, } +#[derive(Serialize, Deserialize, Debug, Clone)] pub enum UploadedDatasetStatus { Available, - Deleted, + Expires(Expiration), + Deleted(Expiration), } #[derive(Debug, FromSql, ToSql)] @@ -104,16 +108,85 @@ pub enum InternalUploadedDatasetStatus { DeletedWithError, } -impl From for UploadedDatasetStatus { - fn from(value: InternalUploadedDatasetStatus) -> Self { - match value { - InternalUploadedDatasetStatus::Available | InternalUploadedDatasetStatus::Expires => { - UploadedDatasetStatus::Available +impl InternalUploadedDatasetStatus { + pub fn convert_to_uploaded_dataset_status( + &self, + dataset_id: &DatasetId, + expiration_timestamp: Option, + dataset_deletion_type: Option, + ) -> Result { + if matches!(self, InternalUploadedDatasetStatus::Available) + && expiration_timestamp.is_none() + && expiration_timestamp.is_none() + { + return Ok(UploadedDatasetStatus::Available); + } else if let Some(deletion_type) = dataset_deletion_type { + let expiration = Expiration { + deletion_timestamp: expiration_timestamp, + deletion_type, + }; + match self { + InternalUploadedDatasetStatus::Available => {} + InternalUploadedDatasetStatus::Expires => { + if expiration_timestamp.is_some() { + return Ok(UploadedDatasetStatus::Expires(expiration)); + } + } + InternalUploadedDatasetStatus::Expired + | InternalUploadedDatasetStatus::UpdateExpired + | InternalUploadedDatasetStatus::Deleted + | InternalUploadedDatasetStatus::DeletedWithError => { + return Ok(UploadedDatasetStatus::Deleted(expiration)); + } } - InternalUploadedDatasetStatus::Expired - | InternalUploadedDatasetStatus::UpdateExpired - | InternalUploadedDatasetStatus::Deleted - | InternalUploadedDatasetStatus::DeletedWithError => UploadedDatasetStatus::Deleted, + } + Err(IllegalDatasetStatus { + dataset: (*dataset_id).into(), + status: "InternalUploadedDatasetStatus is not a legal configuration".to_string(), + }) + } +} + +#[derive(Serialize, Deserialize, Debug, Clone)] +pub enum DatasetType { + UserUpload(UploadedDatasetStatus), + NonUserUpload, +} + +#[derive(Serialize, Deserialize, Debug, Clone)] +pub struct DatasetAccessStatus { + pub id: DatasetId, + pub dataset_type: DatasetType, + pub permissions: Vec, +} + +#[derive(Serialize, Deserialize, Debug, Clone, ToSchema)] +#[serde(rename_all = "camelCase")] +pub struct DatasetAccessStatusResponse { + pub id: DatasetId, + pub is_user_upload: bool, + pub is_available: bool, + pub expiration: Option, + pub permissions: Vec, +} + +impl From for DatasetAccessStatusResponse { + fn from(value: DatasetAccessStatus) -> Self { + let (is_user_upload, is_available, expiration) = match value.dataset_type { + DatasetType::UserUpload(upload) => match upload { + UploadedDatasetStatus::Available => (true, true, None), + UploadedDatasetStatus::Expires(expiration) => (true, true, Some(expiration)), + UploadedDatasetStatus::Deleted(expiration) => (true, false, Some(expiration)), + }, + DatasetType::NonUserUpload => (false, true, None), + }; + + DatasetAccessStatusResponse { + id: value.id, + is_user_upload, + is_available, + expiration, + permissions: value.permissions, } } } @@ -124,6 +197,12 @@ impl From for UploadedDatasetStatus { /// This is because services do not know about database transactions. #[async_trait] pub trait TxUploadedUserDatasetStore { + async fn get_dataset_access_status_in_tx( + &self, + dataset_id: &DatasetId, + tx: &Transaction, + ) -> Result; + async fn validate_expiration_request_in_tx( &self, tx: &Transaction, @@ -145,21 +224,19 @@ pub trait TxUploadedUserDatasetStore { dataset_id: Option<&DatasetId>, ) -> Result<()>; - async fn update_dataset_status_in_tx( + async fn update_uploaded_datasets_status_in_tx( &self, tx: &Transaction, - dataset_id: &DatasetId, + dataset_id: Option<&DatasetId>, ) -> Result<()>; - async fn update_datasets_status_in_tx(&self, tx: &Transaction) -> Result<()>; - - async fn admin_update_datasets_status_in_tx(&self, tx: &Transaction) -> Result<()>; async fn set_expire_for_uploaded_dataset( &self, tx: &Transaction, dataset_id: &DatasetId, expiration: &Expiration, ) -> Result<()>; + async fn unset_expire_for_uploaded_dataset( &self, tx: &Transaction, @@ -179,5 +256,10 @@ pub trait UploadedUserDatasetStore { async fn expire_uploaded_dataset(&self, expire_dataset: ChangeDatasetExpiration) -> Result<()>; + async fn get_dataset_access_status( + &self, + dataset_id: &DatasetId, + ) -> Result; + async fn clear_expired_datasets(&self) -> Result; } diff --git a/services/src/pro/permissions/postgres_permissiondb.rs b/services/src/pro/permissions/postgres_permissiondb.rs index 4a9c53c86..fd53f4d18 100644 --- a/services/src/pro/permissions/postgres_permissiondb.rs +++ b/services/src/pro/permissions/postgres_permissiondb.rs @@ -10,9 +10,10 @@ use crate::pro::permissions::{ }; use async_trait::async_trait; use snafu::{ensure, ResultExt}; +use std::collections::HashSet; use tokio_postgres::{ tls::{MakeTlsConnect, TlsConnect}, - Socket, + Socket, Transaction, }; use uuid::Uuid; @@ -125,6 +126,13 @@ pub trait TxPermissionDb { limit: u32, tx: &tokio_postgres::Transaction<'_>, ) -> Result, PermissionDbError>; + + /// Get all permissions the current user has for this `resource`. + async fn get_user_permissions_in_tx + Send + Sync>( + &self, + resource: R, + tx: &tokio_postgres::Transaction<'_>, + ) -> Result, PermissionDbError>; } #[async_trait] @@ -372,6 +380,44 @@ where Ok(permissions) } + + async fn get_user_permissions_in_tx + Send + Sync>( + &self, + resource: R, + tx: &Transaction<'_>, + ) -> Result, PermissionDbError> { + let resource: ResourceId = resource.into(); + + let stmt = tx + .prepare(&format!( + " + SELECT + p.permission + FROM + permissions p JOIN user_roles r ON (p.role_id = r.role_id) + WHERE + {resource_type} = $1 AND r.user_id = $2 + ;", + resource_type = resource.resource_type_name() + )) + .await + .context(PostgresPermissionDbError)?; + + let rows = tx + .query(&stmt, &[&resource.uuid()?, &self.session.user.id]) + .await + .context(PostgresPermissionDbError)?; + + let mut all_permissions = HashSet::new(); + for row in rows { + let permission: Permission = row.get(0); + for implied_permission in permission.implied_permissions() { + all_permissions.insert(implied_permission); + } + } + + Ok(Vec::from_iter(all_permissions)) + } } #[async_trait] From 617cc8a4df0c88b1b6724ee41b947963f2cc277f Mon Sep 17 00:00:00 2001 From: Nikolaus Glombiewski Date: Thu, 25 Jul 2024 18:07:59 +0200 Subject: [PATCH 11/17] Fix SQL lint; --- services/src/pro/contexts/migrations/current_schema.sql | 7 ++++--- .../migrations/migration_0011_delete_uploaded_datasets.rs | 7 ++++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/services/src/pro/contexts/migrations/current_schema.sql b/services/src/pro/contexts/migrations/current_schema.sql index 00cc4f2e6..239dfafa8 100644 --- a/services/src/pro/contexts/migrations/current_schema.sql +++ b/services/src/pro/contexts/migrations/current_schema.sql @@ -267,7 +267,8 @@ SELECT u.status, u.deletion_type FROM - uploaded_user_datasets u JOIN - user_permitted_datasets p ON (u.user_id = p.user_id) + uploaded_user_datasets AS u INNER JOIN + user_permitted_datasets AS p ON (u.user_id = p.user_id) WHERE - u.expiration <= CURRENT_TIMESTAMP AND (u.status = 'Expires' OR u.status = 'UpdateExpired'); + u.expiration <= CURRENT_TIMESTAMP + AND (u.status = 'Expires' OR u.status = 'UpdateExpired'); diff --git a/services/src/pro/contexts/migrations/migration_0011_delete_uploaded_datasets.rs b/services/src/pro/contexts/migrations/migration_0011_delete_uploaded_datasets.rs index c0f4b0457..b1a483b2b 100644 --- a/services/src/pro/contexts/migrations/migration_0011_delete_uploaded_datasets.rs +++ b/services/src/pro/contexts/migrations/migration_0011_delete_uploaded_datasets.rs @@ -42,10 +42,11 @@ impl ProMigration for ProMigrationImpl { u.status, u.deletion_type FROM - uploaded_user_datasets u JOIN - user_permitted_datasets p ON (u.user_id = p.user_id) + uploaded_user_datasets AS u INNER JOIN + user_permitted_datasets AS p ON (u.user_id = p.user_id) WHERE - u.expiration <= CURRENT_TIMESTAMP AND (u.status = 'Expires' OR u.status = 'UpdateExpired'); + u.expiration <= CURRENT_TIMESTAMP + AND (u.status = 'Expires' OR u.status = 'UpdateExpired'); "#, ) .await?; From 62756de4b83851d5f9cfd176429499fbf3741786 Mon Sep 17 00:00:00 2001 From: Nikolaus Glombiewski Date: Fri, 26 Jul 2024 14:47:48 +0200 Subject: [PATCH 12/17] Wrap test expiration in transaction and increase duration to reduce chance for timing errors; --- services/src/pro/api/handlers/datasets.rs | 4 +- services/src/pro/datasets/postgres.rs | 197 +++++++++++++--------- services/src/pro/datasets/storage.rs | 14 +- services/src/pro/util/tests.rs | 9 + 4 files changed, 142 insertions(+), 82 deletions(-) diff --git a/services/src/pro/api/handlers/datasets.rs b/services/src/pro/api/handlers/datasets.rs index 057da7020..4fd40ad09 100644 --- a/services/src/pro/api/handlers/datasets.rs +++ b/services/src/pro/api/handlers/datasets.rs @@ -772,7 +772,7 @@ mod tests { assert!(status.expiration.is_none()); let current_time = get_db_timestamp(&app_ctx).await; - let future_time = current_time.add(Duration::seconds(2)); + let future_time = current_time.add(Duration::seconds(5)); let expiration = ChangeDatasetExpiration::expire_full(dataset_id, future_time).expiration_change; @@ -800,7 +800,7 @@ mod tests { assert!(status.is_available); assert!(status.expiration.is_some()); - tokio::time::sleep(std::time::Duration::from_secs(2)).await; + tokio::time::sleep(std::time::Duration::from_secs(5)).await; assert!(db.load_dataset(&dataset_id).await.is_err()); diff --git a/services/src/pro/datasets/postgres.rs b/services/src/pro/datasets/postgres.rs index 0d27eff18..1437cf7ac 100644 --- a/services/src/pro/datasets/postgres.rs +++ b/services/src/pro/datasets/postgres.rs @@ -1007,9 +1007,9 @@ where async fn validate_expiration_request_in_tx( &self, - tx: &Transaction, dataset_id: &DatasetId, expiration: &Expiration, + tx: &Transaction, ) -> Result<()> { let (status, deletion_type, legal_expiration): ( InternalUploadedDatasetStatus, @@ -1099,7 +1099,7 @@ where .await .boxed_context(crate::error::PermissionDb)?; - self.update_uploaded_datasets_status_in_tx(tx, Some(dataset_id)) + self.update_uploaded_datasets_status_in_tx(Some(dataset_id), tx) .await?; let stmt = tx @@ -1138,18 +1138,46 @@ where dataset_id: Option<&DatasetId>, ) -> Result<()> { let tx = conn.build_transaction().start().await?; - self.update_uploaded_datasets_status_in_tx(&tx, dataset_id) + self.update_uploaded_datasets_status_in_tx(dataset_id, &tx) .await?; tx.commit().await?; Ok(()) } + async fn expire_uploaded_dataset_in_tx( + &self, + expire_dataset: ChangeDatasetExpiration, + tx: &Transaction, + ) -> Result<()> { + self.ensure_permission_in_tx(expire_dataset.dataset_id.into(), Permission::Owner, tx) + .await + .boxed_context(error::PermissionDb)?; + + self.update_uploaded_datasets_status_in_tx(Some(&expire_dataset.dataset_id), tx) + .await?; + + match expire_dataset.expiration_change { + ExpirationChange::SetExpire(expiration) => { + self.set_expire_for_uploaded_dataset(&expire_dataset.dataset_id, &expiration, tx) + .await?; + } + ExpirationChange::UnsetExpire => { + self.unset_expire_for_uploaded_dataset(&expire_dataset.dataset_id, tx) + .await?; + } + } + self.update_uploaded_datasets_status_in_tx(Some(&expire_dataset.dataset_id), tx) + .await?; + + Ok(()) + } + #[allow(clippy::too_many_lines)] async fn update_uploaded_datasets_status_in_tx( &self, - tx: &Transaction, dataset_id: Option<&DatasetId>, + tx: &Transaction, ) -> Result<()> { fn create_filter( session: &UserSession, @@ -1273,9 +1301,9 @@ where async fn set_expire_for_uploaded_dataset( &self, - tx: &Transaction, dataset_id: &DatasetId, expiration: &Expiration, + tx: &Transaction, ) -> Result<()> { let num_changes = if let Some(delete_timestamp) = expiration.deletion_timestamp { let stmt = tx @@ -1346,7 +1374,7 @@ where }; if num_changes == 0 { - self.validate_expiration_request_in_tx(tx, dataset_id, expiration) + self.validate_expiration_request_in_tx(dataset_id, expiration, tx) .await?; }; @@ -1355,8 +1383,8 @@ where async fn unset_expire_for_uploaded_dataset( &self, - tx: &Transaction, dataset_id: &DatasetId, + tx: &Transaction, ) -> Result<()> { let stmt = tx .prepare( @@ -1495,24 +1523,7 @@ where let mut conn = self.conn_pool.get().await?; let tx = conn.build_transaction().start().await?; - self.ensure_permission_in_tx(expire_dataset.dataset_id.into(), Permission::Owner, &tx) - .await - .boxed_context(error::PermissionDb)?; - - self.update_uploaded_datasets_status_in_tx(&tx, Some(&expire_dataset.dataset_id)) - .await?; - - match expire_dataset.expiration_change { - ExpirationChange::SetExpire(expiration) => { - self.set_expire_for_uploaded_dataset(&tx, &expire_dataset.dataset_id, &expiration) - .await?; - } - ExpirationChange::UnsetExpire => { - self.unset_expire_for_uploaded_dataset(&tx, &expire_dataset.dataset_id) - .await?; - } - } - self.update_uploaded_datasets_status_in_tx(&tx, Some(&expire_dataset.dataset_id)) + self.expire_uploaded_dataset_in_tx(expire_dataset, &tx) .await?; tx.commit().await?; @@ -1543,7 +1554,7 @@ where let mut conn = self.conn_pool.get().await?; let tx = conn.build_transaction().start().await?; - self.update_uploaded_datasets_status_in_tx(&tx, None) + self.update_uploaded_datasets_status_in_tx(None, &tx) .await?; let update_expired = tx @@ -1633,8 +1644,8 @@ mod tests { use crate::error::Error::PermissionDenied; use crate::pro::permissions::{PermissionDb, Role}; use crate::pro::users::{UserCredentials, UserRegistration}; - use crate::pro::util::tests::get_db_timestamp; use crate::pro::util::tests::{admin_login, send_pro_test_request}; + use crate::pro::util::tests::{get_db_timestamp, get_db_timestamp_in_tx}; use crate::util::tests::{SetMultipartBody, TestDataUploads}; use crate::{ contexts::{ApplicationContext, SessionContext}, @@ -1647,7 +1658,7 @@ mod tests { use actix_web::http::header; use actix_web::test; use actix_web_httpauth::headers::authorization::Bearer; - use geoengine_datatypes::primitives::Duration; + use geoengine_datatypes::primitives::{DateTime, Duration}; use geoengine_datatypes::{ collections::VectorDataType, primitives::{CacheTtlSeconds, FeatureDataType, Measurement}, @@ -1991,6 +2002,36 @@ mod tests { .unwrap() } + async fn expire_in_tx_time_duration( + app_ctx: &ProPostgresContext, + user_session: &UserSession, + dataset_id: DatasetId, + fair: bool, + duration: Duration, + ) -> DateTime { + let mut conn = app_ctx.pool.get().await.unwrap(); + let tx = conn.build_transaction().start().await.unwrap(); + + let db = app_ctx.session_context(user_session.clone()).db(); + + let current_time = get_db_timestamp_in_tx(&tx).await; + let future_time = current_time.add(duration); + + let change_dataset_expiration = if fair { + ChangeDatasetExpiration::expire_fair(dataset_id, future_time) + } else { + ChangeDatasetExpiration::expire_full(dataset_id, future_time) + }; + + db.expire_uploaded_dataset_in_tx(change_dataset_expiration, &tx) + .await + .unwrap(); + + tx.commit().await.unwrap(); + + future_time + } + #[ge_context::test] async fn it_lists_datasets_without_tags(app_ctx: ProPostgresContext) { let admin_session = admin_login(&app_ctx).await; @@ -2104,9 +2145,6 @@ mod tests { let mut test_data = TestDataUploads::default(); let user_session = register_test_user(&app_ctx).await; - let current_time = get_db_timestamp(&app_ctx).await; - let future_time = current_time.add(Duration::seconds(3)); - let fair = upload_and_add_point_dataset(&app_ctx, &user_session, "fair", &mut test_data).await; @@ -2120,12 +2158,14 @@ mod tests { tags: None, }; - db.expire_uploaded_dataset(ChangeDatasetExpiration::expire_fair( + expire_in_tx_time_duration( + &app_ctx, + &user_session, fair.dataset_id, - future_time, - )) - .await - .unwrap(); + true, + Duration::seconds(3), + ) + .await; let listing = db .list_datasets(default_list_options.clone()) @@ -2154,10 +2194,6 @@ mod tests { let mut test_data = TestDataUploads::default(); let user_session = register_test_user(&app_ctx).await; - let current_time = get_db_timestamp(&app_ctx).await; - let future_time_1 = current_time.add(Duration::seconds(2)); - let future_time_2 = current_time.add(Duration::seconds(5)); - let fair = upload_and_add_point_dataset(&app_ctx, &user_session, "fair", &mut test_data).await; let fair2full = @@ -2174,30 +2210,38 @@ mod tests { tags: None, }; - db.expire_uploaded_dataset(ChangeDatasetExpiration::expire_fair( + expire_in_tx_time_duration( + &app_ctx, + &user_session, fair.dataset_id, - future_time_1, - )) - .await - .unwrap(); - db.expire_uploaded_dataset(ChangeDatasetExpiration::expire_fair( + true, + Duration::seconds(5), + ) + .await; + expire_in_tx_time_duration( + &app_ctx, + &user_session, fair.dataset_id, - future_time_2, - )) - .await - .unwrap(); - db.expire_uploaded_dataset(ChangeDatasetExpiration::expire_fair( + true, + Duration::seconds(10), + ) + .await; + expire_in_tx_time_duration( + &app_ctx, + &user_session, fair2full.dataset_id, - future_time_1, - )) - .await - .unwrap(); - db.expire_uploaded_dataset(ChangeDatasetExpiration::expire_full( + true, + Duration::seconds(5), + ) + .await; + expire_in_tx_time_duration( + &app_ctx, + &user_session, fair2full.dataset_id, - future_time_1, - )) - .await - .unwrap(); + false, + Duration::seconds(5), + ) + .await; let listing = db .list_datasets(default_list_options.clone()) @@ -2208,7 +2252,7 @@ mod tests { assert!(listing_not_deleted(listing.first().unwrap(), &fair)); assert!(listing_not_deleted(listing.get(1).unwrap(), &fair2full)); - tokio::time::sleep(std::time::Duration::from_secs(3)).await; + tokio::time::sleep(std::time::Duration::from_secs(5)).await; let listing = db .list_datasets(default_list_options.clone()) @@ -2222,7 +2266,7 @@ mod tests { UnknownDatasetId )); - tokio::time::sleep(std::time::Duration::from_secs(3)).await; + tokio::time::sleep(std::time::Duration::from_secs(10)).await; let listing = db .list_datasets(default_list_options.clone()) @@ -2282,17 +2326,18 @@ mod tests { .await .is_err()); - let current_time = get_db_timestamp(&app_ctx).await; - let future_time = current_time.add(Duration::seconds(2)); let fair2available = upload_and_add_point_dataset(&app_ctx, &user_session, "fair2available", &mut test_data) .await; - db.expire_uploaded_dataset(ChangeDatasetExpiration::expire_fair( + + expire_in_tx_time_duration( + &app_ctx, + &user_session, fair2available.dataset_id, - future_time, - )) - .await - .unwrap(); + true, + Duration::seconds(3), + ) + .await; db.expire_uploaded_dataset(ChangeDatasetExpiration::unset_expire( fair2available.dataset_id, )) @@ -2321,8 +2366,6 @@ mod tests { #[ge_context::test] async fn it_handles_dataset_status(app_ctx: ProPostgresContext) { let mut test_data = TestDataUploads::default(); - let current_time = get_db_timestamp(&app_ctx).await; - let future_time = current_time.add(Duration::seconds(3)); let volume_dataset = add_test_volume_dataset(&app_ctx).await; @@ -2348,12 +2391,14 @@ mod tests { )); has_read_and_owner_permissions(&access_status.permissions); - db.expire_uploaded_dataset(ChangeDatasetExpiration::expire_fair( + let future_time = expire_in_tx_time_duration( + &app_ctx, + &user_session, user_dataset, - future_time, - )) - .await - .unwrap(); + true, + Duration::seconds(3), + ) + .await; let access_status = db.get_dataset_access_status(&user_dataset).await.unwrap(); assert!(matches!( access_status.dataset_type, diff --git a/services/src/pro/datasets/storage.rs b/services/src/pro/datasets/storage.rs index bf8ddff67..403590478 100644 --- a/services/src/pro/datasets/storage.rs +++ b/services/src/pro/datasets/storage.rs @@ -205,9 +205,9 @@ pub trait TxUploadedUserDatasetStore { async fn validate_expiration_request_in_tx( &self, - tx: &Transaction, dataset_id: &DatasetId, expiration: &Expiration, + tx: &Transaction, ) -> Result<()>; async fn uploaded_dataset_status_in_tx( @@ -224,23 +224,29 @@ pub trait TxUploadedUserDatasetStore { dataset_id: Option<&DatasetId>, ) -> Result<()>; - async fn update_uploaded_datasets_status_in_tx( + async fn expire_uploaded_dataset_in_tx( &self, + expire_dataset: ChangeDatasetExpiration, tx: &Transaction, + ) -> Result<()>; + + async fn update_uploaded_datasets_status_in_tx( + &self, dataset_id: Option<&DatasetId>, + tx: &Transaction, ) -> Result<()>; async fn set_expire_for_uploaded_dataset( &self, - tx: &Transaction, dataset_id: &DatasetId, expiration: &Expiration, + tx: &Transaction, ) -> Result<()>; async fn unset_expire_for_uploaded_dataset( &self, - tx: &Transaction, dataset_id: &DatasetId, + tx: &Transaction, ) -> Result<()>; } diff --git a/services/src/pro/util/tests.rs b/services/src/pro/util/tests.rs index bb7576cd4..562a85f07 100644 --- a/services/src/pro/util/tests.rs +++ b/services/src/pro/util/tests.rs @@ -42,6 +42,8 @@ use geoengine_operators::{ }; use tokio::runtime::Handle; use tokio_postgres::NoTls; +#[cfg(test)] +use tokio_postgres::Transaction; use super::config::{Cache, Quota}; @@ -458,6 +460,13 @@ pub async fn get_db_timestamp(app_ctx: &ProPostgresContext) -> DateTime { conn.query_one(&get_time_stmt, &[]).await.unwrap().get(0) } +#[cfg(test)] +#[allow(clippy::missing_panics_doc)] +pub async fn get_db_timestamp_in_tx(tx: &Transaction<'_>) -> DateTime { + let get_time_stmt = tx.prepare("SELECT CURRENT_TIMESTAMP;").await.unwrap(); + tx.query_one(&get_time_stmt, &[]).await.unwrap().get(0) +} + #[cfg(test)] pub(in crate::pro) mod mock_oidc { use crate::pro::users::{DefaultJsonWebKeySet, DefaultProviderMetadata}; From 496ae6076ee18b593462015359adb4527876af82 Mon Sep 17 00:00:00 2001 From: Nikolaus Glombiewski Date: Wed, 31 Jul 2024 14:35:46 +0200 Subject: [PATCH 13/17] Add check if dataset is a user upload; --- services/src/pro/datasets/postgres.rs | 80 +++++++++++++++++---------- services/src/pro/datasets/storage.rs | 2 + 2 files changed, 53 insertions(+), 29 deletions(-) diff --git a/services/src/pro/datasets/postgres.rs b/services/src/pro/datasets/postgres.rs index 1437cf7ac..0ef727682 100644 --- a/services/src/pro/datasets/postgres.rs +++ b/services/src/pro/datasets/postgres.rs @@ -794,42 +794,40 @@ where let mut conn = self.conn_pool.get().await?; let tx = conn.build_transaction().start().await?; - let _uploaded = self.uploaded_dataset_status_in_tx(&dataset_id, &tx).await; - if let Err(error) = _uploaded { - if matches!(error, UnknownDatasetId) { - self.ensure_permission_in_tx(dataset_id.into(), Permission::Owner, &tx) - .await - .boxed_context(crate::error::PermissionDb)?; + let is_user_upload = self.is_user_upload_in_tx(&dataset_id, &tx).await?; + if !is_user_upload { + self.ensure_permission_in_tx(dataset_id.into(), Permission::Owner, &tx) + .await + .boxed_context(crate::error::PermissionDb)?; - let stmt = tx - .prepare( - " - SELECT - TRUE - FROM - user_permitted_datasets p JOIN datasets d - ON (p.dataset_id = d.id) - WHERE - d.id = $1 AND p.user_id = $2 AND p.permission = 'Owner';", - ) - .await?; + let stmt = tx + .prepare( + " + SELECT + TRUE + FROM + user_permitted_datasets p JOIN datasets d + ON (p.dataset_id = d.id) + WHERE + d.id = $1 AND p.user_id = $2 AND p.permission = 'Owner';", + ) + .await?; - let rows = tx - .query(&stmt, &[&dataset_id, &self.session.user.id]) - .await?; + let rows = tx + .query(&stmt, &[&dataset_id, &self.session.user.id]) + .await?; - if rows.is_empty() { - return Err(Error::OperationRequiresOwnerPermission); - } + if rows.is_empty() { + return Err(Error::OperationRequiresOwnerPermission); + } - let stmt = tx.prepare("DELETE FROM datasets WHERE id = $1;").await?; + let stmt = tx.prepare("DELETE FROM datasets WHERE id = $1;").await?; - tx.execute(&stmt, &[&dataset_id]).await?; + tx.execute(&stmt, &[&dataset_id]).await?; - tx.commit().await?; + tx.commit().await?; - return Ok(()); - } + return Ok(()); } self.expire_uploaded_dataset(ChangeDatasetExpiration::delete_full(dataset_id)) @@ -952,6 +950,30 @@ where >::TlsConnect: Send, <>::TlsConnect as TlsConnect>::Future: Send, { + async fn is_user_upload_in_tx(&self, dataset_id: &DatasetId, tx: &Transaction) -> Result { + self.ensure_permission_in_tx((*dataset_id).into(), Permission::Read, tx) + .await + .boxed_context(crate::error::PermissionDb)?; + + let stmt = tx + .prepare( + " + SELECT + TRUE + FROM + uploaded_user_datasets + WHERE + dataset_id = $1;", + ) + .await?; + + let result = tx + .query_opt(&stmt, &[&dataset_id, &self.session.user.id]) + .await?; + + return Ok(result.is_some()); + } + async fn get_dataset_access_status_in_tx( &self, dataset_id: &DatasetId, diff --git a/services/src/pro/datasets/storage.rs b/services/src/pro/datasets/storage.rs index 403590478..bdeeaf224 100644 --- a/services/src/pro/datasets/storage.rs +++ b/services/src/pro/datasets/storage.rs @@ -197,6 +197,8 @@ impl From for DatasetAccessStatusResponse { /// This is because services do not know about database transactions. #[async_trait] pub trait TxUploadedUserDatasetStore { + async fn is_user_upload_in_tx(&self, dataset_id: &DatasetId, tx: &Transaction) -> Result; + async fn get_dataset_access_status_in_tx( &self, dataset_id: &DatasetId, From 7b3f778097784e504e38c43ed0af92a4e68d37c8 Mon Sep 17 00:00:00 2001 From: Nikolaus Glombiewski Date: Wed, 31 Jul 2024 14:42:47 +0200 Subject: [PATCH 14/17] Rename fair upload deletion migration; --- ...d_datasets.rs => migration_0012_fair_upload_deletion.rs} | 4 ++-- services/src/contexts/migrations/mod.rs | 6 +++--- services/src/contexts/mod.rs | 2 +- ...d_datasets.rs => migration_0012_fair_upload_deletion.rs} | 4 ++-- services/src/pro/contexts/migrations/mod.rs | 6 +++--- 5 files changed, 11 insertions(+), 11 deletions(-) rename services/src/contexts/migrations/{migration_0010_delete_uploaded_datasets.rs => migration_0012_fair_upload_deletion.rs} (83%) rename services/src/pro/contexts/migrations/{migration_0011_delete_uploaded_datasets.rs => migration_0012_fair_upload_deletion.rs} (92%) diff --git a/services/src/contexts/migrations/migration_0010_delete_uploaded_datasets.rs b/services/src/contexts/migrations/migration_0012_fair_upload_deletion.rs similarity index 83% rename from services/src/contexts/migrations/migration_0010_delete_uploaded_datasets.rs rename to services/src/contexts/migrations/migration_0012_fair_upload_deletion.rs index 136a850fe..dcd61eea8 100644 --- a/services/src/contexts/migrations/migration_0010_delete_uploaded_datasets.rs +++ b/services/src/contexts/migrations/migration_0012_fair_upload_deletion.rs @@ -6,10 +6,10 @@ use crate::error::Result; use super::database_migration::{DatabaseVersion, Migration}; /// This migration adds new delete options for uploaded user datasets -pub struct Migration0011DeleteUploadedDatasets; +pub struct Migration0012FairUploadDeletion; #[async_trait] -impl Migration for Migration0011DeleteUploadedDatasets { +impl Migration for Migration0012FairUploadDeletion { fn prev_version(&self) -> Option { Some("0010_s2_stac_time_buffers".into()) } diff --git a/services/src/contexts/migrations/mod.rs b/services/src/contexts/migrations/mod.rs index 022b7e823..8a73372f8 100644 --- a/services/src/contexts/migrations/mod.rs +++ b/services/src/contexts/migrations/mod.rs @@ -9,7 +9,7 @@ pub use crate::contexts::migrations::{ migration_0007_owner_role::Migration0007OwnerRole, migration_0008_band_names::Migration0008BandNames, migration_0009_oidc_tokens::Migration0009OidcTokens, - migration_0010_delete_uploaded_datasets::Migration0011DeleteUploadedDatasets, + migration_0012_fair_upload_deletion::Migration0012FairUploadDeletion, migration_0010_s2_stack_time_buffers::Migration0010S2StacTimeBuffers, }; pub use database_migration::{ @@ -28,7 +28,7 @@ mod migration_0006_ebv_provider; pub mod migration_0007_owner_role; pub mod migration_0008_band_names; pub mod migration_0009_oidc_tokens; -pub mod migration_0010_delete_uploaded_datasets; +pub mod migration_0012_fair_upload_deletion; pub mod migration_0010_s2_stack_time_buffers; #[cfg(test)] @@ -54,7 +54,7 @@ pub fn all_migrations() -> Vec> { Box::new(Migration0008BandNames), Box::new(Migration0009OidcTokens), Box::new(Migration0010S2StacTimeBuffers), - Box::new(Migration0011DeleteUploadedDatasets), + Box::new(Migration0012FairUploadDeletion), ] } diff --git a/services/src/contexts/mod.rs b/services/src/contexts/mod.rs index fa013e35b..88a3193f8 100644 --- a/services/src/contexts/mod.rs +++ b/services/src/contexts/mod.rs @@ -29,7 +29,7 @@ pub use migrations::{ Migration0002DatasetListingProvider, Migration0003GbifConfig, Migration0004DatasetListingProviderPrio, Migration0005GbifColumnSelection, Migration0006EbvProvider, Migration0007OwnerRole, Migration0008BandNames, - Migration0009OidcTokens, Migration0010S2StacTimeBuffers, Migration0011DeleteUploadedDatasets, + Migration0009OidcTokens, Migration0010S2StacTimeBuffers, Migration0012FairUploadDeletion, MigrationResult, }; pub use postgres::{PostgresContext, PostgresDb, PostgresSessionContext}; diff --git a/services/src/pro/contexts/migrations/migration_0011_delete_uploaded_datasets.rs b/services/src/pro/contexts/migrations/migration_0012_fair_upload_deletion.rs similarity index 92% rename from services/src/pro/contexts/migrations/migration_0011_delete_uploaded_datasets.rs rename to services/src/pro/contexts/migrations/migration_0012_fair_upload_deletion.rs index b1a483b2b..4f49b4b83 100644 --- a/services/src/pro/contexts/migrations/migration_0011_delete_uploaded_datasets.rs +++ b/services/src/pro/contexts/migrations/migration_0012_fair_upload_deletion.rs @@ -2,10 +2,10 @@ use async_trait::async_trait; use tokio_postgres::Transaction; use super::database_migration::{ProMigration, ProMigrationImpl}; -use crate::{contexts::Migration0011DeleteUploadedDatasets, error::Result}; +use crate::{contexts::Migration0012FairUploadDeletion, error::Result}; #[async_trait] -impl ProMigration for ProMigrationImpl { +impl ProMigration for ProMigrationImpl { async fn pro_migrate(&self, tx: &Transaction<'_>) -> Result<()> { tx.batch_execute( r#" diff --git a/services/src/pro/contexts/migrations/mod.rs b/services/src/pro/contexts/migrations/mod.rs index e11156b9b..2a4a37107 100644 --- a/services/src/pro/contexts/migrations/mod.rs +++ b/services/src/pro/contexts/migrations/mod.rs @@ -4,7 +4,7 @@ use crate::contexts::{ Migration0003GbifConfig, Migration0004DatasetListingProviderPrio, Migration0005GbifColumnSelection, Migration0006EbvProvider, Migration0007OwnerRole, Migration0008BandNames, Migration0009OidcTokens, Migration0010S2StacTimeBuffers, - Migration0011DeleteUploadedDatasets, + Migration0012FairUploadDeletion, }; use crate::pro::contexts::migrations::database_migration::NoProMigrationImpl; @@ -17,7 +17,7 @@ mod migration_0004_dataset_listing_provider_prio; mod migration_0007_owner_role; mod migration_0009_oidc_tokens; mod migration_0010_s2_stack_time_buffers; -mod migration_0011_delete_uploaded_datasets; +mod migration_0012_fair_upload_deletion; /// Get all regular and pro migrations. This function wraps all regular migrations into a pro migration. pub fn pro_migrations() -> Vec> @@ -39,7 +39,7 @@ where Box::new(NoProMigrationImpl::from(Migration0008BandNames)), Box::new(ProMigrationImpl::from(Migration0009OidcTokens)), Box::new(ProMigrationImpl::from(Migration0010S2StacTimeBuffers)), - Box::new(ProMigrationImpl::from(Migration0011DeleteUploadedDatasets)), + Box::new(ProMigrationImpl::from(Migration0012FairUploadDeletion)), ] } From ad9c7c99ee7564f3fd327c2745fb27d8fc762e6f Mon Sep 17 00:00:00 2001 From: Nikolaus Glombiewski Date: Wed, 31 Jul 2024 14:56:35 +0200 Subject: [PATCH 15/17] Fix formatting; --- services/src/pro/contexts/mod.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/services/src/pro/contexts/mod.rs b/services/src/pro/contexts/mod.rs index 04dd8fc19..ba5d98029 100644 --- a/services/src/pro/contexts/mod.rs +++ b/services/src/pro/contexts/mod.rs @@ -44,7 +44,10 @@ pub trait ProApplicationContext: ApplicationContext + Use fn oidc_manager(&self) -> &OidcManager; } -pub trait ProGeoEngineDb: GeoEngineDb + UserDb + PermissionDb + RoleDb + UploadedUserDatasetStore {} +pub trait ProGeoEngineDb: + GeoEngineDb + UserDb + PermissionDb + RoleDb + UploadedUserDatasetStore +{ +} pub struct ExecutionContextImpl where From 8bdab563ef4a9156f1be493ec41d874f1e9ae6c8 Mon Sep 17 00:00:00 2001 From: Nikolaus Glombiewski Date: Wed, 31 Jul 2024 16:31:24 +0200 Subject: [PATCH 16/17] Fix sql parameters; --- services/src/pro/datasets/postgres.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/src/pro/datasets/postgres.rs b/services/src/pro/datasets/postgres.rs index 01a7224d8..e0506b98c 100644 --- a/services/src/pro/datasets/postgres.rs +++ b/services/src/pro/datasets/postgres.rs @@ -914,7 +914,7 @@ where .await?; let result = tx - .query_opt(&stmt, &[&dataset_id, &self.session.user.id]) + .query_opt(&stmt, &[&dataset_id]) .await?; return Ok(result.is_some()); From f0fcf62038c289ab79f8a480a0290d72879015d2 Mon Sep 17 00:00:00 2001 From: Nikolaus Glombiewski Date: Wed, 31 Jul 2024 16:41:54 +0200 Subject: [PATCH 17/17] Fix formatting; --- services/src/pro/datasets/postgres.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/services/src/pro/datasets/postgres.rs b/services/src/pro/datasets/postgres.rs index e0506b98c..2e468acb2 100644 --- a/services/src/pro/datasets/postgres.rs +++ b/services/src/pro/datasets/postgres.rs @@ -913,9 +913,7 @@ where ) .await?; - let result = tx - .query_opt(&stmt, &[&dataset_id]) - .await?; + let result = tx.query_opt(&stmt, &[&dataset_id]).await?; return Ok(result.is_some()); }