feat(drive): impl policy photo + music policies
add `include_in_photo_index` and `include_in_music_index` policies
both true for default personal drive
photo is implemented
music is not yet implemented
This commit is contained in:
Edouard Vanbelle
@@ -448,7 +448,7 @@ impl DriveManagementService {
|
||||
&self,
|
||||
caller_id: Uuid,
|
||||
drive_id: Uuid,
|
||||
partial: crate::domain::entities::drive::DrivePolicies,
|
||||
partial: serde_json::Value,
|
||||
) -> Result<crate::domain::entities::drive::DrivePolicies, DomainError> {
|
||||
let merged = self
|
||||
.drive_repo
|
||||
@@ -474,6 +474,8 @@ impl DriveManagementService {
|
||||
forbid_public_links = merged.forbid_public_links,
|
||||
forbid_cross_drive_move = merged.forbid_cross_drive_move,
|
||||
forbid_owner_role_change = merged.forbid_owner_role_change,
|
||||
include_in_photo_index = merged.include_in_photo_index,
|
||||
include_in_music_index = merged.include_in_music_index,
|
||||
"📜 drive policies updated",
|
||||
);
|
||||
Ok(merged)
|
||||
|
||||
@@ -4,22 +4,29 @@ use uuid::Uuid;
|
||||
|
||||
use crate::application::dtos::geo_dto::{GeoBounds, GeoCluster};
|
||||
use crate::common::errors::DomainError;
|
||||
use crate::domain::services::authorization::Subject;
|
||||
use crate::infrastructure::repositories::pg::FileBlobReadRepository;
|
||||
use crate::infrastructure::services::pg_acl_engine::PgAclEngine;
|
||||
|
||||
/// "Places" use case: the caller's geotagged photos aggregated into map
|
||||
/// clusters.
|
||||
///
|
||||
/// Strictly user-scoped — the repository filters `WHERE fi.user_id = $1`, so,
|
||||
/// like [`RecentService`](super::recent_service::RecentService) and the photos
|
||||
/// timeline, it needs no `AuthorizationEngine` check: the `caller_id`
|
||||
/// parameter *is* the access scope.
|
||||
/// Post-§15 the surface follows the Photos scope: default personal drive
|
||||
/// + drives where `policies.include_in_photo_index = true` AND caller
|
||||
/// has Read. The repository query joins `role_grants` on the drive
|
||||
/// resource type; group-mediated grants are honoured via the caller
|
||||
/// expansion done here.
|
||||
pub struct PlacesService {
|
||||
file_read: Arc<FileBlobReadRepository>,
|
||||
authorization: Arc<PgAclEngine>,
|
||||
}
|
||||
|
||||
impl PlacesService {
|
||||
pub fn new(file_read: Arc<FileBlobReadRepository>) -> Self {
|
||||
Self { file_read }
|
||||
pub fn new(file_read: Arc<FileBlobReadRepository>, authorization: Arc<PgAclEngine>) -> Self {
|
||||
Self {
|
||||
file_read,
|
||||
authorization,
|
||||
}
|
||||
}
|
||||
|
||||
/// Aggregation cell side, in degrees, for a slippy-map zoom level. The
|
||||
@@ -30,7 +37,8 @@ impl PlacesService {
|
||||
360.0 / (2_f64.powi(z) * 4.0)
|
||||
}
|
||||
|
||||
/// Clustered geotagged photos for `caller_id` within `bounds`.
|
||||
/// Clustered geotagged photos in the caller's Photos-scope drive set,
|
||||
/// within `bounds`.
|
||||
pub async fn clusters(
|
||||
&self,
|
||||
caller_id: Uuid,
|
||||
@@ -38,8 +46,12 @@ impl PlacesService {
|
||||
zoom: u8,
|
||||
) -> Result<Vec<GeoCluster>, DomainError> {
|
||||
let cell = Self::cell_for_zoom(zoom);
|
||||
let (subject_types, subject_ids) = self
|
||||
.authorization
|
||||
.expand_subject_for_listing(Subject::User(caller_id))
|
||||
.await?;
|
||||
self.file_read
|
||||
.list_geo_clusters(caller_id, bounds, cell)
|
||||
.list_geo_clusters(&subject_types, &subject_ids, bounds, cell)
|
||||
.await
|
||||
}
|
||||
}
|
||||
|
||||
+11
-3
@@ -912,12 +912,20 @@ impl AppServiceFactory {
|
||||
}
|
||||
|
||||
/// Creates the Places (photo map) service. Reuses the existing file-read
|
||||
/// repository — the data is the caller's own geotagged photos.
|
||||
/// repository — the data is the caller's Photos-scope geotagged photos
|
||||
/// (§15: default personal drive + drives with
|
||||
/// `include_in_photo_index = true` AND caller has Read).
|
||||
/// `authorization` is used for the same subject expansion that
|
||||
/// `photos_handler::list_photos` runs.
|
||||
pub fn create_places_service(
|
||||
&self,
|
||||
file_read: &Arc<FileBlobReadRepository>,
|
||||
authorization: &Arc<crate::infrastructure::services::pg_acl_engine::PgAclEngine>,
|
||||
) -> Arc<PlacesService> {
|
||||
let service = Arc::new(PlacesService::new(file_read.clone()));
|
||||
let service = Arc::new(PlacesService::new(
|
||||
file_read.clone(),
|
||||
authorization.clone(),
|
||||
));
|
||||
tracing::info!("Places service initialized");
|
||||
service
|
||||
}
|
||||
@@ -1285,7 +1293,7 @@ impl AppServiceFactory {
|
||||
apps.recent_service = Some(recent_service_eager.clone());
|
||||
|
||||
places_service = if core.config.features.enable_places {
|
||||
Some(self.create_places_service(&repos.file_read_repository))
|
||||
Some(self.create_places_service(&repos.file_read_repository, &authorization))
|
||||
} else {
|
||||
None
|
||||
};
|
||||
|
||||
@@ -181,6 +181,26 @@ pub struct DrivePolicies {
|
||||
/// writes) and `::remove_member` (refuses Owner removals) when the
|
||||
/// caller is non-admin.
|
||||
pub forbid_owner_role_change: bool,
|
||||
/// Opts this drive into the `/api/photos` timeline (§15). Non-default
|
||||
/// drives are omitted by default so a random shared folder full of
|
||||
/// screenshots doesn't bleed into the personal timeline; owners flip
|
||||
/// this on when the drive genuinely is a photo library (e.g. "Family
|
||||
/// Photos"). Default personal drives get `true` on creation via the
|
||||
/// `PersonalDriveLifecycleHook` + a one-shot backfill for existing
|
||||
/// rows, so the SQL predicate is a single positive rule with no
|
||||
/// per-kind carve-out. Read at `file_blob_read_repository::
|
||||
/// list_media_files` + `list_geo_clusters`. See §15 for the query
|
||||
/// shape and rationale.
|
||||
pub include_in_photo_index: bool,
|
||||
/// Same shape as `include_in_photo_index`, applied to the Music
|
||||
/// library surface (playlists today; a `/api/music/tracks` library
|
||||
/// view later). Symmetric opt-in — Music was originally cross-drive
|
||||
/// via a `forbid_music_index` opt-out, but that mixed-form naming
|
||||
/// created "one include-in, one forbid" confusion and the
|
||||
/// "shared audio is always intentional" claim didn't hold under
|
||||
/// scrutiny (voicemail MP3s in a work drive shouldn't bleed into
|
||||
/// the personal library). See §15.
|
||||
pub include_in_music_index: bool,
|
||||
}
|
||||
|
||||
impl DrivePolicies {
|
||||
|
||||
@@ -256,10 +256,18 @@ pub trait DriveRepository: Send + Sync + 'static {
|
||||
///
|
||||
/// Caller is responsible for the `Manage` permission check; this
|
||||
/// method does not re-verify.
|
||||
///
|
||||
/// `partial` is a raw JSON object carrying **only** the keys the
|
||||
/// caller wants to change — the repo passes it verbatim to the
|
||||
/// `policies || $partial` JSONB merge. Using the typed
|
||||
/// `DrivePolicies` here would serialise every field (including
|
||||
/// unset ones as `false`) and clobber other flags on the row;
|
||||
/// keeping the merge on the raw `Value` preserves the
|
||||
/// partial-update semantic the handler documents.
|
||||
async fn update_policies(
|
||||
&self,
|
||||
drive_id: Uuid,
|
||||
partial: &crate::domain::entities::drive::DrivePolicies,
|
||||
partial: &serde_json::Value,
|
||||
) -> Result<crate::domain::entities::drive::DrivePolicies, DriveRepositoryError>;
|
||||
}
|
||||
|
||||
|
||||
@@ -116,11 +116,22 @@ impl DriveRepository for DrivePgRepository {
|
||||
.map_err(|e| Self::map_sqlx_err("create_personal_drive_atomic.begin", e))?;
|
||||
|
||||
// 1. Drive row (root_folder_id NULL — populated in step 3).
|
||||
//
|
||||
// Default personal drives are seeded with `include_in_photo_index`
|
||||
// + `include_in_music_index` = true so the Photos / Music
|
||||
// predicates (§15) can be a single positive rule keyed off the
|
||||
// JSONB flag — no per-kind carve-out needed at query time. Any
|
||||
// future admin PATCH toggling either flag off shows a confirm
|
||||
// dialog in the UI (unusual action; empties the user's Photos
|
||||
// timeline / Music library).
|
||||
let drive_id: Uuid = sqlx::query_scalar(
|
||||
r#"
|
||||
INSERT INTO storage.drives
|
||||
(kind, default_for_user, quota_bytes, policies)
|
||||
VALUES ('personal', $1, $2, '{}'::jsonb)
|
||||
VALUES (
|
||||
'personal', $1, $2,
|
||||
'{"include_in_photo_index": true, "include_in_music_index": true}'::jsonb
|
||||
)
|
||||
RETURNING id
|
||||
"#,
|
||||
)
|
||||
@@ -636,16 +647,17 @@ impl DriveRepository for DrivePgRepository {
|
||||
async fn update_policies(
|
||||
&self,
|
||||
drive_id: Uuid,
|
||||
partial: &crate::domain::entities::drive::DrivePolicies,
|
||||
partial: &serde_json::Value,
|
||||
) -> Result<crate::domain::entities::drive::DrivePolicies, DriveRepositoryError> {
|
||||
// JSONB-level merge (`||`) keeps unknown keys already on disk —
|
||||
// the column remains the canonical bag (see
|
||||
// `DrivePolicies::from_value` — typed read is lenient, untyped
|
||||
// write is preserving). RETURNING surfaces the post-merge bag so
|
||||
// the audit log shows what the row actually carries afterwards.
|
||||
let partial_json = serde_json::to_value(partial).map_err(|e| {
|
||||
DriveRepositoryError::StorageError(format!("serialise partial policies: {e}"))
|
||||
})?;
|
||||
// write is preserving). The caller passes a raw `Value` with
|
||||
// ONLY the keys it wants to change (never a full `DrivePolicies`
|
||||
// round-trip, which would serialise all-false defaults into the
|
||||
// merge and clobber other flags). RETURNING surfaces the
|
||||
// post-merge bag so the audit log shows what the row actually
|
||||
// carries afterwards.
|
||||
let row: Option<(serde_json::Value,)> = sqlx::query_as(
|
||||
"UPDATE storage.drives \
|
||||
SET policies = policies || $2, \
|
||||
@@ -654,7 +666,7 @@ impl DriveRepository for DrivePgRepository {
|
||||
RETURNING policies",
|
||||
)
|
||||
.bind(drive_id)
|
||||
.bind(&partial_json)
|
||||
.bind(partial)
|
||||
.fetch_optional(self.pool.as_ref())
|
||||
.await
|
||||
.map_err(|e| Self::map_sqlx_err("update_policies", e))?;
|
||||
|
||||
@@ -445,15 +445,36 @@ impl FileBlobReadRepository {
|
||||
///
|
||||
/// Uses the denormalised `media_sort_date` column (synced from
|
||||
/// `file_metadata.captured_at` by trigger) so no JOIN with
|
||||
/// `file_metadata` is needed. The partial index
|
||||
/// `idx_files_media_timeline` covers the full query: filter + ORDER BY
|
||||
/// in a single Index Scan — O(LIMIT) not O(N).
|
||||
/// `file_metadata` is needed. The partial covering index
|
||||
/// `idx_files_media_timeline_by_drive` (migration 20260901000001)
|
||||
/// keys on `(drive_id, media_sort_date DESC)` filtered on non-trashed
|
||||
/// image/video rows — Postgres does one IndexScan per in-scope
|
||||
/// drive_id already ordered by capture date, so LIMIT stops the scan
|
||||
/// early. Same O(LIMIT) shape as the pre-D7 `user_id`-keyed hot path.
|
||||
///
|
||||
/// Scope (`docs/plan/drive.md` §15): the caller's *effective subjects*
|
||||
/// × drives with `policies.include_in_photo_index = true`. Default
|
||||
/// personal drives always match because the flag is materialised to
|
||||
/// `true` at drive creation (see
|
||||
/// `DriveRepository::create_personal_drive_atomic` + the backfill
|
||||
/// migration `20260901000000_default_personal_photo_music_flags.sql`)
|
||||
/// — no per-kind carve-out needed. Non-default drives (secondary
|
||||
/// personals, shared drives) surface here only after their owner
|
||||
/// flips the flag on via the admin "Manage policies" modal.
|
||||
///
|
||||
/// `subject_types` / `subject_ids` are the caller expanded through
|
||||
/// their group memberships (`AuthorizationEngine::
|
||||
/// expand_subject_for_listing`); the arrays reach into the ANY()
|
||||
/// predicates so a group-mediated grant on a drive counts too.
|
||||
pub async fn list_media_files(
|
||||
&self,
|
||||
owner_id: Uuid,
|
||||
subject_types: &[&str],
|
||||
subject_ids: &[Uuid],
|
||||
before: Option<i64>,
|
||||
limit: i64,
|
||||
) -> Result<(Vec<File>, Vec<i64>, Vec<(Option<i32>, Option<i32>)>), DomainError> {
|
||||
let subject_types_owned: Vec<String> =
|
||||
subject_types.iter().map(|s| s.to_string()).collect();
|
||||
let rows: Vec<MediaFileRow> = sqlx::query_as(
|
||||
r#"
|
||||
SELECT fi.id::text, fi.name, fi.folder_id::text, fo.path,
|
||||
@@ -468,16 +489,27 @@ impl FileBlobReadRepository {
|
||||
FROM storage.files fi
|
||||
LEFT JOIN storage.folders fo ON fo.id = fi.folder_id
|
||||
LEFT JOIN storage.file_metadata fm ON fm.file_id = fi.id
|
||||
WHERE fi.user_id = $1
|
||||
WHERE fi.drive_id IN (
|
||||
SELECT d.id
|
||||
FROM storage.drives d
|
||||
JOIN storage.role_grants g
|
||||
ON g.resource_type = 'drive'
|
||||
AND g.resource_id = d.id
|
||||
WHERE g.subject_type = ANY($1)
|
||||
AND g.subject_id = ANY($2)
|
||||
AND (g.expires_at IS NULL OR g.expires_at > NOW())
|
||||
AND (d.policies->>'include_in_photo_index')::boolean = true
|
||||
)
|
||||
AND NOT fi.is_trashed
|
||||
AND (fi.mime_type LIKE 'image/%' OR fi.mime_type LIKE 'video/%')
|
||||
AND ($2::bigint IS NULL
|
||||
OR EXTRACT(EPOCH FROM fi.media_sort_date)::bigint < $2::bigint)
|
||||
AND ($3::bigint IS NULL
|
||||
OR EXTRACT(EPOCH FROM fi.media_sort_date)::bigint < $3::bigint)
|
||||
ORDER BY fi.media_sort_date DESC
|
||||
LIMIT $3
|
||||
LIMIT $4
|
||||
"#,
|
||||
)
|
||||
.bind(owner_id)
|
||||
.bind(&subject_types_owned)
|
||||
.bind(subject_ids)
|
||||
.bind(before)
|
||||
.bind(limit)
|
||||
.fetch_all(self.pool.as_ref())
|
||||
@@ -500,15 +532,26 @@ impl FileBlobReadRepository {
|
||||
}
|
||||
|
||||
/// Aggregate the caller's geotagged photos into grid cells of side `cell`
|
||||
/// (degrees) within `bounds`. Plain SQL (no PostGIS), scoped to `user_id`.
|
||||
/// Returns one cluster per non-empty cell with its centroid, photo count
|
||||
/// and a representative photo id (for the cluster thumbnail).
|
||||
/// (degrees) within `bounds`. Plain SQL (no PostGIS).
|
||||
///
|
||||
/// Scope: same `include_in_photo_index` predicate as
|
||||
/// `list_media_files` (§15). Places is the map view over the same
|
||||
/// content set the Photos timeline shows, so the two surfaces MUST
|
||||
/// agree on drive scope. If a drive is opt-out for Photos its
|
||||
/// geotagged files never appear on the map either.
|
||||
///
|
||||
/// This query is a per-cell aggregate (group by rounded lat/lng
|
||||
/// bucket) rather than an ORDER BY / LIMIT hot path — the plain
|
||||
/// `idx_files_drive_id` is sufficient to seek by drive.
|
||||
pub async fn list_geo_clusters(
|
||||
&self,
|
||||
user_id: Uuid,
|
||||
subject_types: &[&str],
|
||||
subject_ids: &[Uuid],
|
||||
bounds: GeoBounds,
|
||||
cell: f64,
|
||||
) -> Result<Vec<GeoCluster>, DomainError> {
|
||||
let subject_types_owned: Vec<String> =
|
||||
subject_types.iter().map(|s| s.to_string()).collect();
|
||||
let rows: Vec<(i64, f64, f64, String)> = sqlx::query_as(
|
||||
r#"
|
||||
SELECT count(*) AS n,
|
||||
@@ -517,16 +560,27 @@ impl FileBlobReadRepository {
|
||||
min(fm.file_id::text) AS sample_id
|
||||
FROM storage.file_metadata fm
|
||||
JOIN storage.files fi ON fi.id = fm.file_id
|
||||
WHERE fi.user_id = $1
|
||||
WHERE fi.drive_id IN (
|
||||
SELECT d.id
|
||||
FROM storage.drives d
|
||||
JOIN storage.role_grants g
|
||||
ON g.resource_type = 'drive'
|
||||
AND g.resource_id = d.id
|
||||
WHERE g.subject_type = ANY($1)
|
||||
AND g.subject_id = ANY($2)
|
||||
AND (g.expires_at IS NULL OR g.expires_at > NOW())
|
||||
AND (d.policies->>'include_in_photo_index')::boolean = true
|
||||
)
|
||||
AND NOT fi.is_trashed
|
||||
AND fm.latitude IS NOT NULL
|
||||
AND fm.longitude IS NOT NULL
|
||||
AND fm.longitude BETWEEN $2 AND $3
|
||||
AND fm.latitude BETWEEN $4 AND $5
|
||||
GROUP BY round(fm.longitude / $6), round(fm.latitude / $6)
|
||||
AND fm.longitude BETWEEN $3 AND $4
|
||||
AND fm.latitude BETWEEN $5 AND $6
|
||||
GROUP BY round(fm.longitude / $7), round(fm.latitude / $7)
|
||||
"#,
|
||||
)
|
||||
.bind(user_id)
|
||||
.bind(&subject_types_owned)
|
||||
.bind(subject_ids)
|
||||
.bind(bounds.west)
|
||||
.bind(bounds.east)
|
||||
.bind(bounds.south)
|
||||
|
||||
@@ -416,6 +416,10 @@ pub struct UpdateDrivePoliciesDto {
|
||||
pub forbid_cross_drive_move: Option<bool>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub forbid_owner_role_change: Option<bool>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub include_in_photo_index: Option<bool>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub include_in_music_index: Option<bool>,
|
||||
}
|
||||
|
||||
/// `PATCH /api/drives/{id}/policies` — **OxiCloud-admin only** policy
|
||||
@@ -494,18 +498,22 @@ pub async fn update_drive_policies(
|
||||
serde_json::Value::Bool(v),
|
||||
);
|
||||
}
|
||||
if let Some(v) = dto.include_in_photo_index {
|
||||
partial_obj.insert("include_in_photo_index".into(), serde_json::Value::Bool(v));
|
||||
}
|
||||
if let Some(v) = dto.include_in_music_index {
|
||||
partial_obj.insert("include_in_music_index".into(), serde_json::Value::Bool(v));
|
||||
}
|
||||
// Pass the raw JSON straight through so the JSONB `||` merge in
|
||||
// the repo only touches keys the caller supplied. Round-tripping
|
||||
// via `DrivePolicies` (which has `#[serde(default)]`) would
|
||||
// silently fill every omitted field with `false` — the merge
|
||||
// would then clobber every unmentioned policy on the row.
|
||||
let partial_value = serde_json::Value::Object(partial_obj);
|
||||
let partial: crate::domain::entities::drive::DrivePolicies =
|
||||
match serde_json::from_value(partial_value) {
|
||||
Ok(p) => p,
|
||||
Err(e) => {
|
||||
return AppError::bad_request(format!("invalid policy body: {e}")).into_response();
|
||||
}
|
||||
};
|
||||
|
||||
match state
|
||||
.drive_management_service
|
||||
.update_policies(auth_user.id, drive_id, partial)
|
||||
.update_policies(auth_user.id, drive_id, partial_value)
|
||||
.await
|
||||
{
|
||||
Ok(merged) => (StatusCode::OK, axum::Json(merged)).into_response(),
|
||||
|
||||
@@ -12,6 +12,8 @@ use tracing::{error, info};
|
||||
use crate::application::dtos::file_dto::FileDto;
|
||||
use crate::application::dtos::geo_dto::GeoBounds;
|
||||
use crate::common::di::AppState;
|
||||
use crate::domain::services::authorization::Subject;
|
||||
use crate::interfaces::errors::AppError;
|
||||
use crate::interfaces::middleware::auth::AuthUser;
|
||||
|
||||
/// Query parameters for the photos timeline endpoint.
|
||||
@@ -63,13 +65,29 @@ pub async fn list_photos(
|
||||
headers: HeaderMap,
|
||||
Query(params): Query<PhotosQueryParams>,
|
||||
) -> impl IntoResponse {
|
||||
let user_id = auth_user.id;
|
||||
let caller_id = auth_user.id;
|
||||
let limit = params.limit.unwrap_or(200).clamp(1, 500);
|
||||
|
||||
// Expand the caller into (subject_types, subject_ids) so group-mediated
|
||||
// drive memberships surface in the Photos timeline too. Mirrors what
|
||||
// `drive_handler::list_drives` and `trash_service::list_resources_paged`
|
||||
// already do — one call to the AuthZ engine per request.
|
||||
let (subject_types, subject_ids) = match state
|
||||
.authorization
|
||||
.expand_subject_for_listing(Subject::User(caller_id))
|
||||
.await
|
||||
{
|
||||
Ok(pair) => pair,
|
||||
Err(e) => {
|
||||
error!("list_photos: subject expansion failed: {e}");
|
||||
return AppError::from(e).into_response();
|
||||
}
|
||||
};
|
||||
|
||||
let file_read = &state.repositories.file_read_repository;
|
||||
|
||||
match file_read
|
||||
.list_media_files(user_id, params.before, limit)
|
||||
.list_media_files(&subject_types, &subject_ids, params.before, limit)
|
||||
.await
|
||||
{
|
||||
Ok((files, sort_dates, dims)) => {
|
||||
|
||||
Reference in New Issue
Block a user