-
Notifications
You must be signed in to change notification settings - Fork 35
feat: adding gc endpoint #1996
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: adding gc endpoint #1996
Conversation
Reviewer's GuideIntroduces a new GET /api/v2/purl/gc endpoint in the purl service that invokes the gc_purls method to perform garbage collection of orphaned packages, complete with route registration, API documentation, and an integration test. Sequence diagram for the new garbage collection endpointsequenceDiagram
participant Client
participant "API Server"
participant "PurlService"
participant "Database"
Client->>"API Server": GET /api/v2/purl/gc
"API Server"->>"PurlService": gc_purls(db)
"PurlService"->>"Database": Query for orphaned packages
"Database"-->>"PurlService": List of orphaned packages
"PurlService"->>"Database": Delete orphaned packages
"Database"-->>"PurlService": Deletion result
"PurlService"-->>"API Server": GC result (string)
"API Server"-->>Client: 200 OK (GC result)
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes - here's some feedback:
- The OpenAPI spec registers the GC response as text/plain but the handler returns JSON, so update the documented content type or change the response to plain text to keep them in sync.
- Garbage collection mutates data, so consider using a write-level permission guard instead of Require to prevent unauthorized access.
- Double-check that the route path in the handler ("/v2/purl/gc") aligns with how your service is mounted under "/api" to avoid unexpected 404 errors.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The OpenAPI spec registers the GC response as text/plain but the handler returns JSON, so update the documented content type or change the response to plain text to keep them in sync.
- Garbage collection mutates data, so consider using a write-level permission guard instead of Require<ReadSbom> to prevent unauthorized access.
- Double-check that the route path in the handler ("/v2/purl/gc") aligns with how your service is mounted under "/api" to avoid unexpected 404 errors.
## Individual Comments
### Comment 1
<location> `modules/fundamental/src/purl/endpoints/mod.rs:85-93` </location>
<code_context>
Ok(HttpResponse::Ok().json(service.purls(search, paginated, db.as_ref()).await?))
}
+#[utoipa::path(
+ operation_id = "garbageCollect",
+ tag = "purl",
+ responses(
+ (status = 200, description = "Performs garbage collection for orphaned packages", body = String),
+ ),
+)]
+#[get("/v2/purl/gc")]
+pub async fn gc(
+ service: web::Data<PurlService>,
+ db: web::Data<Database>,
</code_context>
<issue_to_address>
**issue:** The gc endpoint currently returns a JSON-encoded string, but the OpenAPI spec expects text/plain.
Use HttpResponse::Ok().body(...) to return plain text, or update the OpenAPI spec to expect application/json for this endpoint.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
jcrossley3
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Just one minor nit.
1d399d3 to
e8122df
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1996 +/- ##
==========================================
- Coverage 68.00% 67.75% -0.25%
==========================================
Files 355 355
Lines 19807 19816 +9
Branches 19807 19816 +9
==========================================
- Hits 13470 13427 -43
- Misses 5557 5613 +56
+ Partials 780 776 -4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
e8122df to
bbb69bd
Compare
|
Backport failed for Please cherry-pick the changes locally and resolve any conflicts. git fetch origin release/0.3.z
git worktree add -d .worktree/backport-1996-to-release/0.3.z origin/release/0.3.z
cd .worktree/backport-1996-to-release/0.3.z
git switch --create backport-1996-to-release/0.3.z
git cherry-pick -x 9d0f3ac52b6560ff412cd1a945d991446a85a457 |
Related to #1980 (comment)
Summary by Sourcery
Introduce a new garbage collection endpoint for orphaned PURLs
New Features:
Documentation:
Tests: