[OPIK-6264] [BE] feat: add environments CRUD API#6563
Conversation
Backend Tests - Integration Group 5147 tests 144 ✅ 3m 0s ⏱️ Results for commit 5acf158. ♻️ This comment has been updated with latest results. |
LifeXplorer
left a comment
There was a problem hiding this comment.
Nice work! Added few non-blocking comments
| @ApiResponse(responseCode = "200", description = "Environments page", content = @Content(schema = @Schema(implementation = Environment.EnvironmentPage.class))) | ||
| }) | ||
| @JsonView({Environment.View.Public.class}) | ||
| public Response find() { |
There was a problem hiding this comment.
find() pretends to paginate but doesn't
Environment.EnvironmentPage carries page/size/total/sortableBy, but:
- EnvironmentsResource.find() accepts no @QueryParam (page, size, name, sorting, filters).
- Service hardcodes page=1, total=environments.size().
- DAO findAll has no LIMIT.
There was a problem hiding this comment.
Yes, basically we want to return all environments. We are capping it at 20 max. I used page structure for consistency and potential future expansion.
| @ApiResponse(responseCode = "409", description = "Conflict", content = @Content(schema = @Schema(implementation = ErrorMessage.class))) | ||
| }) | ||
| @RateLimited | ||
| public Response update(@PathParam("id") @NotNull UUID id, |
There was a problem hiding this comment.
PATCH semantics around empty/null aren't nailed down.
EnvironmentUpdate uses @Size(max=500) on description (and @Size(max=20) on color) without @NotBlank — and EnvironmentDAO.update does description = COALESCE(:description, description). So:
- null → "no change" ✓
- "" → sets to empty string (passes validation, COALESCE doesn't treat "" as null in MySQL).
There's no way to clear description back to NULL. Decide the contract:
- "Null means no-op, empty means clear" — document and add a test.
- "Empty is a no-op" — reject empty with @pattern(regexp = ".+") or a custom not-blank-when-present.
There was a problem hiding this comment.
Right now we can remove description by setting it to empty string, this is intended behavior. Not sure if we need to update it to NULL. There is no use case for it. If user wants to remove, FE will pass empty string and it will be effectively removed, DB will hold empty string.
Details
Adds a workspace-scoped Environments CRUD module to the backend so users can manage named environments (with description, color, position) via the private API. Introduces the
environmentsMySQL table, JDBI DAO, transactional service, and Dropwizard resource, plus a configurable per-workspace cap (default 20) and the same audit/analytics/locking patterns used by other workspace entities./v1/private/environments:GET /(list, capped),GET /{id},POST /(create, rate-limited),PATCH /{id}(partial update, rate-limited),POST /delete(idempotent batch delete).Public/Write), snake_case naming, and validation (name regex^[A-Za-z0-9_-]+$, length caps 150/500/20).LockServicemutex, maps unique-constraint violations to409 Conflict, emits anenvironment_createdanalytics event, and usesREAD_ONLY/WRITEtransaction templates.COALESCEfor partial PATCH updates and scopes every query byworkspace_id.environmentswith a(workspace_id, name)unique constraint.OPIK_ENVIRONMENT_MAX_PER_WORKSPACE(default 20) in config.yml.Change checklist
Issues
AI-WATERMARK
AI-WATERMARK: no
Testing
409), workspace cap (409), validation errors (400), missing-id (404), and PATCH partial semantics.cd apps/opik-backend && mvn test -Dtest=EnvironmentsResourceTest.Documentation
N/A — OpenAPI annotations on the resource cover the new endpoints; no separate docs added.