mirror of https://github.com/buster-so/buster.git
317 lines
9.2 KiB
Markdown
317 lines
9.2 KiB
Markdown
|
---
|
||
|
title: Remove Metrics from Collection REST Endpoint
|
||
|
author: Cascade
|
||
|
date: 2025-03-19
|
||
|
status: Draft
|
||
|
---
|
||
|
|
||
|
# Remove Metrics from Collection REST Endpoint
|
||
|
|
||
|
## Problem Statement
|
||
|
|
||
|
Users need the ability to programmatically remove metrics from collections via a REST API. Currently, this functionality is not available, limiting the ability to manage collections through the API.
|
||
|
|
||
|
## Goals
|
||
|
|
||
|
1. Create a REST endpoint to remove metrics from a collection
|
||
|
2. Implement proper permission validation
|
||
|
3. Ensure data integrity with proper error handling
|
||
|
4. Follow established patterns for REST endpoints and handlers
|
||
|
|
||
|
## Non-Goals
|
||
|
|
||
|
1. Modifying the existing collections functionality
|
||
|
2. Creating UI components for this endpoint
|
||
|
|
||
|
## Technical Design
|
||
|
|
||
|
### REST Endpoint
|
||
|
|
||
|
**Endpoint:** `DELETE /collections/:id/metrics`
|
||
|
|
||
|
**Request Body:**
|
||
|
```json
|
||
|
{
|
||
|
"metric_ids": ["uuid1", "uuid2", "uuid3"]
|
||
|
}
|
||
|
```
|
||
|
|
||
|
**Response:**
|
||
|
- `200 OK` - Success
|
||
|
```json
|
||
|
{
|
||
|
"message": "Metrics removed from collection successfully"
|
||
|
}
|
||
|
```
|
||
|
- `400 Bad Request` - Invalid input
|
||
|
- `403 Forbidden` - Insufficient permissions
|
||
|
- `404 Not Found` - Collection not found
|
||
|
- `500 Internal Server Error` - Server error
|
||
|
|
||
|
### Handler Implementation
|
||
|
|
||
|
The handler will:
|
||
|
1. Validate that the collection exists
|
||
|
2. Check if the user has appropriate permissions (Owner, FullAccess, or CanEdit)
|
||
|
3. Mark the specified metrics as deleted in the `collections_to_assets` table
|
||
|
|
||
|
### File Changes
|
||
|
|
||
|
#### New Files
|
||
|
|
||
|
1. `libs/handlers/src/collections/remove_metrics_from_collection_handler.rs`
|
||
|
```rust
|
||
|
use anyhow::{anyhow, Result};
|
||
|
use database::{
|
||
|
enums::{AssetPermissionRole, AssetType, IdentityType},
|
||
|
pool::get_pg_pool,
|
||
|
schema::{collections, collections_to_assets},
|
||
|
};
|
||
|
use diesel::{ExpressionMethods, QueryDsl};
|
||
|
use diesel_async::RunQueryDsl;
|
||
|
use sharing::check_asset_permission::has_permission;
|
||
|
use tracing::{error, info};
|
||
|
use uuid::Uuid;
|
||
|
|
||
|
/// Removes metrics from a collection
|
||
|
///
|
||
|
/// # Arguments
|
||
|
///
|
||
|
/// * `collection_id` - The unique identifier of the collection
|
||
|
/// * `metric_ids` - Vector of metric IDs to remove from the collection
|
||
|
/// * `user_id` - The unique identifier of the user performing the action
|
||
|
///
|
||
|
/// # Returns
|
||
|
///
|
||
|
/// Ok(()) on success, or an error if the operation fails
|
||
|
pub async fn remove_metrics_from_collection_handler(
|
||
|
collection_id: &Uuid,
|
||
|
metric_ids: Vec<Uuid>,
|
||
|
user_id: &Uuid,
|
||
|
) -> Result<()> {
|
||
|
info!(
|
||
|
collection_id = %collection_id,
|
||
|
user_id = %user_id,
|
||
|
metric_count = metric_ids.len(),
|
||
|
"Removing metrics from collection"
|
||
|
);
|
||
|
|
||
|
if metric_ids.is_empty() {
|
||
|
return Ok(());
|
||
|
}
|
||
|
|
||
|
// 1. Validate the collection exists
|
||
|
let mut conn = get_pg_pool().get().await.map_err(|e| {
|
||
|
error!("Database connection error: {}", e);
|
||
|
anyhow!("Failed to get database connection: {}", e)
|
||
|
})?;
|
||
|
|
||
|
let collection_exists = collections::table
|
||
|
.filter(collections::id.eq(collection_id))
|
||
|
.filter(collections::deleted_at.is_null())
|
||
|
.count()
|
||
|
.get_result::<i64>(&mut conn)
|
||
|
.await
|
||
|
.map_err(|e| {
|
||
|
error!("Error checking if collection exists: {}", e);
|
||
|
anyhow!("Database error: {}", e)
|
||
|
})?;
|
||
|
|
||
|
if collection_exists == 0 {
|
||
|
error!(
|
||
|
collection_id = %collection_id,
|
||
|
"Collection not found"
|
||
|
);
|
||
|
return Err(anyhow!("Collection not found"));
|
||
|
}
|
||
|
|
||
|
// 2. Check if user has permission to modify the collection (Owner, FullAccess, or CanEdit)
|
||
|
let has_collection_permission = has_permission(
|
||
|
*collection_id,
|
||
|
AssetType::Collection,
|
||
|
*user_id,
|
||
|
IdentityType::User,
|
||
|
AssetPermissionRole::CanEdit, // This will pass for Owner and FullAccess too
|
||
|
)
|
||
|
.await
|
||
|
.map_err(|e| {
|
||
|
error!(
|
||
|
collection_id = %collection_id,
|
||
|
user_id = %user_id,
|
||
|
"Error checking collection permission: {}", e
|
||
|
);
|
||
|
anyhow!("Error checking permissions: {}", e)
|
||
|
})?;
|
||
|
|
||
|
if !has_collection_permission {
|
||
|
error!(
|
||
|
collection_id = %collection_id,
|
||
|
user_id = %user_id,
|
||
|
"User does not have permission to modify this collection"
|
||
|
);
|
||
|
return Err(anyhow!("User does not have permission to modify this collection"));
|
||
|
}
|
||
|
|
||
|
// 3. Mark metrics as deleted in the collection
|
||
|
let now = chrono::Utc::now();
|
||
|
let updated = diesel::update(collections_to_assets::table)
|
||
|
.filter(collections_to_assets::collection_id.eq(collection_id))
|
||
|
.filter(collections_to_assets::asset_id.eq_any(&metric_ids))
|
||
|
.filter(collections_to_assets::asset_type.eq(AssetType::MetricFile))
|
||
|
.filter(collections_to_assets::deleted_at.is_null())
|
||
|
.set((
|
||
|
collections_to_assets::deleted_at.eq(now),
|
||
|
collections_to_assets::updated_at.eq(now),
|
||
|
collections_to_assets::updated_by.eq(user_id),
|
||
|
))
|
||
|
.execute(&mut conn)
|
||
|
.await
|
||
|
.map_err(|e| {
|
||
|
error!("Error removing metrics from collection: {}", e);
|
||
|
anyhow!("Database error: {}", e)
|
||
|
})?;
|
||
|
|
||
|
info!(
|
||
|
collection_id = %collection_id,
|
||
|
user_id = %user_id,
|
||
|
metric_count = metric_ids.len(),
|
||
|
updated_count = updated,
|
||
|
"Successfully removed metrics from collection"
|
||
|
);
|
||
|
|
||
|
Ok(())
|
||
|
}
|
||
|
|
||
|
#[cfg(test)]
|
||
|
mod tests {
|
||
|
use super::*;
|
||
|
use database::enums::{AssetPermissionRole, AssetType, IdentityType};
|
||
|
use uuid::Uuid;
|
||
|
|
||
|
#[tokio::test]
|
||
|
async fn test_remove_metrics_from_collection_handler() {
|
||
|
// This is a placeholder for the actual test
|
||
|
// In a real implementation, we would use test fixtures and a test database
|
||
|
assert!(true);
|
||
|
}
|
||
|
}
|
||
|
```
|
||
|
|
||
|
2. `src/routes/rest/routes/collections/remove_metrics_from_collection.rs`
|
||
|
```rust
|
||
|
use axum::{
|
||
|
extract::{Extension, Json, Path},
|
||
|
http::StatusCode,
|
||
|
};
|
||
|
use handlers::collections::remove_metrics_from_collection_handler;
|
||
|
use middleware::AuthenticatedUser;
|
||
|
use serde::Deserialize;
|
||
|
use tracing::info;
|
||
|
use uuid::Uuid;
|
||
|
|
||
|
use crate::routes::rest::ApiResponse;
|
||
|
|
||
|
#[derive(Debug, Deserialize)]
|
||
|
pub struct RemoveMetricsRequest {
|
||
|
pub metric_ids: Vec<Uuid>,
|
||
|
}
|
||
|
|
||
|
/// REST handler for removing metrics from a collection
|
||
|
///
|
||
|
/// # Arguments
|
||
|
///
|
||
|
/// * `user` - The authenticated user making the request
|
||
|
/// * `id` - The unique identifier of the collection
|
||
|
/// * `request` - The metric IDs to remove from the collection
|
||
|
///
|
||
|
/// # Returns
|
||
|
///
|
||
|
/// A success message on success, or an appropriate error response
|
||
|
pub async fn remove_metrics_from_collection_rest_handler(
|
||
|
Extension(user): Extension<AuthenticatedUser>,
|
||
|
Path(id): Path<Uuid>,
|
||
|
Json(request): Json<RemoveMetricsRequest>,
|
||
|
) -> Result<ApiResponse<String>, (StatusCode, String)> {
|
||
|
info!(
|
||
|
collection_id = %id,
|
||
|
user_id = %user.id,
|
||
|
metric_count = request.metric_ids.len(),
|
||
|
"Processing DELETE request to remove metrics from collection"
|
||
|
);
|
||
|
|
||
|
match remove_metrics_from_collection_handler(&id, request.metric_ids, &user.id).await {
|
||
|
Ok(_) => Ok(ApiResponse::JsonData("Metrics removed from collection successfully".to_string())),
|
||
|
Err(e) => {
|
||
|
tracing::error!("Error removing metrics from collection: {}", e);
|
||
|
|
||
|
// Map specific errors to appropriate status codes
|
||
|
let error_message = e.to_string();
|
||
|
|
||
|
if error_message.contains("Collection not found") {
|
||
|
return Err((StatusCode::NOT_FOUND, format!("Collection not found: {}", e)));
|
||
|
} else if error_message.contains("permission") {
|
||
|
return Err((StatusCode::FORBIDDEN, format!("Insufficient permissions: {}", e)));
|
||
|
}
|
||
|
|
||
|
Err((StatusCode::INTERNAL_SERVER_ERROR, format!("Failed to remove metrics from collection: {}", e)))
|
||
|
}
|
||
|
}
|
||
|
}
|
||
|
```
|
||
|
|
||
|
3. Update `libs/handlers/src/collections/mod.rs` to include the new handler
|
||
|
4. Update `src/routes/rest/routes/collections/mod.rs` to include the new endpoint
|
||
|
|
||
|
### Database Operations
|
||
|
|
||
|
The implementation will use the `collections_to_assets` table with the following operations:
|
||
|
1. SELECT to check if the collection exists
|
||
|
2. UPDATE to mark records as deleted
|
||
|
|
||
|
## Testing Strategy
|
||
|
|
||
|
### Unit Tests
|
||
|
|
||
|
1. Test the handler with mocked database connections
|
||
|
- Test removing metrics from a collection
|
||
|
- Test error cases (collection not found, insufficient permissions)
|
||
|
- Test removing metrics that aren't in the collection
|
||
|
|
||
|
2. Test the REST endpoint
|
||
|
- Test successful request
|
||
|
- Test error responses for various scenarios
|
||
|
|
||
|
### Integration Tests
|
||
|
|
||
|
1. Test the endpoint with a test database
|
||
|
- Create a collection and add metrics to it
|
||
|
- Remove metrics from the collection
|
||
|
- Verify the database state
|
||
|
- Test with different user roles
|
||
|
|
||
|
## Security Considerations
|
||
|
|
||
|
- The endpoint requires authentication
|
||
|
- Permission checks ensure users can only modify collections they have access to
|
||
|
- Input validation prevents malicious data
|
||
|
|
||
|
## Monitoring and Logging
|
||
|
|
||
|
- All operations are logged with appropriate context
|
||
|
- Errors are logged with detailed information
|
||
|
|
||
|
## Dependencies
|
||
|
|
||
|
- `libs/sharing` - For permission checking
|
||
|
- `libs/database` - For database operations
|
||
|
|
||
|
## Rollout Plan
|
||
|
|
||
|
1. Implement the handler and endpoint
|
||
|
2. Write tests
|
||
|
3. Code review
|
||
|
4. Deploy to staging
|
||
|
5. Test in staging
|
||
|
6. Deploy to production
|
||
|
7. Monitor for issues
|