feat(artistRole): CRUD API for artistRole#60
Merged
Conversation
- Add language field to ArtistRole Prisma model - Implement core module: DTOs, repository (with soft-delete), service (with outbox), DI container - Add REST handlers: GET /artist-role/:id, POST /artist-role, PATCH /artist-role/:id, DELETE /artist-role/:id - Add search service extending ISearchService with single-language indexing - Add search handler: GET /artist-roles?q= - Write unit tests (7), integration tests (9), E2E tests (11) - Remove rate limiting from login handler - Refactor search interface and update search services for artist/singer/song
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR introduces a complete artist-role CRUD feature with REST endpoints and search integration, refactors search services to support per-language localized indexing, removes rate-limiting from auth handlers, and adds data import tooling for bulk loading catalog entities. ChangesAuth Middleware Cleanup
Artist Role CRUD Feature
Search Service Architecture Refactoring
Supporting Infrastructure Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🧹 Nitpick comments (3)
apps/backend/src/handlers/catalog/artistRole/search.ts (2)
21-23: ⚡ Quick winBound and normalize
qbefore hitting search.Adding
.trim().max(...)reduces avoidable expensive queries and keeps request validation defensive.Suggested patch
query: z.object({ - q: z.string().optional(), + q: z.string().trim().max(256).optional(), }),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/backend/src/handlers/catalog/artistRole/search.ts` around lines 21 - 23, The request schema for the search handler accepts a raw q string without bounds or normalization; update the z.object query schema in search.ts (the query definition that contains q) to trim and enforce a max length (e.g. z.string().trim().max(...).optional()) so incoming q is normalized and bounded, and in the search handler use the normalized value (the trimmed/validated q) before calling the search routine to avoid expensive unbounded queries.
6-24: ⚡ Quick winReplace the TODO with an explicit response schema for this endpoint.
Right now this route is undocumented on the response side, which makes client generation and contract checks weaker than the other artist-role endpoints.
If you want, I can draft a concrete
ArtistRoleSearchResponseSchemaand wire it into this handler.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/backend/src/handlers/catalog/artistRole/search.ts` around lines 6 - 24, Add an explicit response schema for artistRoleSearchHandler by defining and using a DTO/validation schema (e.g., ArtistRoleSearchResponseSchema) that describes the 200 response shape (likely an array of artist-role objects or a wrapper with items and metadata) and attach it to the handler options under a response key (for example response: { 200: ArtistRoleSearchResponseSchema }). Ensure the schema matches the objects returned by artistRoleSearchService.search and update any import/exports to wire ArtistRoleSearchResponseSchema into the handler options so client generation and contract checks can validate the endpoint.packages/core/tests/unit/ArtistRoleService.test.ts (1)
81-86: ⚡ Quick winConsolidate the NOT_FOUND assertions to avoid calling the service 3 times
Bun’s
expect(promise).rejects...usage in this repo is not awaited (e.g.,OutboxProcessor.test.ts/OutboxService.test.ts), so the missingawaithere isn’t the real problem. The tests are still invokinggetDetails/update/deletemultiple times; capture the promise once per test and run the threerejectsmatchers against it.Suggested fix
test("throws NOT_FOUND error when role does not exist", async () => { mockRepository.getDetailsById.mockResolvedValueOnce(null); - expect(artistRoleService.getDetails(999)).rejects.toThrow(AppError); - expect(artistRoleService.getDetails(999)).rejects.toThrow("error.artistRole.notfound"); - expect(artistRoleService.getDetails(999)).rejects.toMatchObject({ + const promise = artistRoleService.getDetails(999); + expect(promise).rejects.toThrow(AppError); + expect(promise).rejects.toThrow("error.artistRole.notfound"); + expect(promise).rejects.toMatchObject({ code: "NOT_FOUND", statusCode: 404, }); }); @@ test("throws NOT_FOUND error when role does not exist", async () => { mockRepository.getById.mockResolvedValueOnce(null); - expect(artistRoleService.update(999, updateInput)).rejects.toThrow(AppError); - expect(artistRoleService.update(999, updateInput)).rejects.toThrow( + const promise = artistRoleService.update(999, updateInput); + expect(promise).rejects.toThrow(AppError); + expect(promise).rejects.toThrow( "error.artistRole.notfound" ); - expect(artistRoleService.update(999, updateInput)).rejects.toMatchObject({ + expect(promise).rejects.toMatchObject({ code: "NOT_FOUND", statusCode: 404, }); }); @@ test("throws NOT_FOUND error when role does not exist", async () => { mockRepository.getById.mockResolvedValueOnce(null); - expect(artistRoleService.delete(999)).rejects.toThrow(AppError); - expect(artistRoleService.delete(999)).rejects.toThrow("error.artistRole.notfound"); - expect(artistRoleService.delete(999)).rejects.toMatchObject({ + const promise = artistRoleService.delete(999); + expect(promise).rejects.toThrow(AppError); + expect(promise).rejects.toThrow("error.artistRole.notfound"); + expect(promise).rejects.toMatchObject({ code: "NOT_FOUND", statusCode: 404, }); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/tests/unit/ArtistRoleService.test.ts` around lines 81 - 86, The test currently calls artistRoleService.getDetails(999) three times; capture the single returned promise (e.g., const promise = artistRoleService.getDetails(999)) and reuse that promise for all three rejects assertions (expect(promise).rejects.toThrow(AppError); expect(promise).rejects.toThrow("error.artistRole.notfound"); expect(promise).rejects.toMatchObject({ code: "NOT_FOUND", statusCode: 404 });) so the service is invoked only once and all matchers run against the same rejected promise.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/backend/src/handlers/auth/login.ts`:
- Around line 17-62: The login endpoint (loginHandler for POST "/session")
currently lacks throttling and needs per-IP and per-identifier rate limiting
reinstated: add a rate-limit check/middleware into the request pipeline for
loginHandler (e.g., immediately before the handler where ip() is used) that
enforces a strict per-IP limit and a per-identifier (email/username) limit,
return appropriate 429 responses on exceed, and document the control in the
route detail; ensure the check uses the incoming headers/body to key
identifier-based limits and integrates with your existing rate-limit provider or
an in-memory/fallback store so the protection is enforced at the application
level if not guaranteed at the edge.
In `@apps/backend/src/handlers/auth/signup.ts`:
- Around line 18-60: The signup route lost rate-limiting—reintroduce route-level
throttling on the POST "/user" handler (signupHandler) by applying a
rate-limit/throttle middleware (or middleware factory) to the Elysia instance or
directly to the post("/user") route; configure it to key by IP (use the existing
ip() middleware or headers), set a conservative limit (e.g., small requests per
minute and a short ban/penalty window), and ensure the middleware returns
appropriate 429 responses and is included in the route options so automated
signups are rate-limited while legitimate users are unaffected.
In `@apps/backend/src/handlers/catalog/artistRole/create.ts`:
- Around line 28-31: The response map for the create route in
apps/backend/src/handlers/catalog/artistRole/create.ts is missing a 400 entry
for validation errors; update the response object (the response map used by the
create handler) to include 400: ErrorResponseSchema alongside 201:
ArtistRoleResponseSchema and 401: ErrorResponseSchema so that body validation
failures are documented in the OpenAPI contract.
In `@packages/core/src/modules/auth/betterAuth.ts`:
- Around line 56-60: The better-auth logger's log implementation forwards raw
variadic args into appLogger[level] as metadata (logger.log -> appLogger[level]
with { metadata: args }), which can leak sensitive auth fields to Pino/OTEL;
sanitize or allowlist the metadata before passing it on: inspect the incoming
...args in logger.log, strip or redact sensitive keys (authorization, cookie,
password, token, etc.) or build an explicit allowlist of safe fields, then call
appLogger[level] with { metadata: sanitizedMetadata } (and ensure any OTEL emit
path receives only the sanitized/allowlisted attributes rather than the raw
args).
In `@packages/core/src/modules/catalog/artistRole/container.ts`:
- Line 9: The embedding service address is hardcoded in the creation of
embeddingManager via treaty<EmbeddingApp>("localhost:14900"); change this to
read the endpoint from configuration/environment (e.g.,
process.env.EMBEDDING_SERVICE_URL or your app config accessor) and validate it
before use; if the value is missing or invalid, throw an error or exit early so
the process fails fast. Update the code that constructs embeddingManager to use
the resolved variable and include a clear error message referencing
embeddingManager/treaty<EmbeddingApp> when aborting.
In `@packages/core/src/modules/catalog/artistRole/dto.ts`:
- Around line 6-16: CreateArtistRoleRequestSchema and
UpdateArtistRoleRequestSchema currently allow empty/whitespace names and Update
accepts an empty object; fix by validating name to be non-empty after trimming
and preventing empty update payloads: change name in
CreateArtistRoleRequestSchema to require a non-blank string (e.g., use
z.string().refine(s => s.trim().length > 0, "name cannot be empty") or
z.string().min(1).transform(s => s.trim()).refine(s => s.length>0,...)), and in
UpdateArtistRoleRequestSchema ensure any provided name uses the same non-blank
validation and add a top-level refinement on UpdateArtistRoleRequestSchema
(e.g., .refine(obj => Object.keys(obj).length > 0, "update payload must contain
at least one field") ) so totally-empty update objects are rejected.
In `@packages/core/src/modules/catalog/artistRole/repository.ts`:
- Around line 50-67: The update and softDelete methods currently mutate rows by
id only, allowing modifications to soft-deleted records; update the where clause
in both ArtistRoleRepository.update and ArtistRoleRepository.softDelete (the
calls inside query("db.artistRole.update", ...) and
query("db.artistRole.softDelete", ...)) to include deletedAt: null so the Prisma
client calls client.artistRole.update({ where: { id, deletedAt: null }, ... })
only affect non-deleted rows (keep the existing data/payload and tx/client
usage).
In `@packages/core/src/modules/catalog/artistRole/service.ts`:
- Around line 50-88: The pre-check using repository.getById(id) should be moved
inside the same prisma.$transaction as the mutation to avoid a race; in both
update and delete wrap a transaction that first calls repository.getById(id, tx)
and throws the same AppError if null, then performs repository.update(id, input,
tx) (for update) or repository.softDelete(id, tx) (for delete), then calls
outbox.createEntry(..., tx) and returns the entry (and role for update); keep
outbox.enqueue(entry) outside the transaction as before. Ensure you call the
repository.getById method that accepts the transaction/tx parameter and preserve
the existing AppError message and types.
In `@packages/core/src/outbox/queue.ts`:
- Line 13: The debug log call using appLogger.debug(`Connecting to redis at
${env.REDIS_URL}`) can leak credentials; update the code that logs the Redis
connection to redact or avoid printing env.REDIS_URL directly — for example,
parse env.REDIS_URL and log only safe parts (hostname and port) or replace
credentials with a mask (e.g., show "redis://****`@host`:port" or use URL.hostname
and URL.port). Locate the appLogger.debug usage and change it to log the
sanitized URL or explicit host/port instead of the raw env.REDIS_URL.
In `@packages/core/src/search/catalog/artistRole.ts`:
- Around line 23-30: The embedding call in artist-role indexing
(embeddingManager.embeddings.post inside the code that builds the returned
object with _vectors via this.buildVectors) can throw and currently aborts
indexing; wrap the embedding call in a try/catch so that if the embeddings
request fails you log/debug the error and set _vectors: null (instead of calling
this.buildVectors with undefined), then continue returning the same object (id,
name, localizedNames) so the document is indexed without vectors; reference
embeddingManager.embeddings.post, this.buildVectors, and _vectors in your
change.
In `@packages/core/src/search/catalog/singer.ts`:
- Around line 33-35: The document payload for Singer is storing the source
locale via singer.language instead of the indexed locale parameter, causing
mismatch with the target index; update the object construction in the singer
document creation to use the function parameter language (not singer.language)
for the language field so the written document in the singer_${language} index
reflects the indexed locale—locate the code that sets name: this.getName(singer,
language), description: this.getDescription(singer, language) and replace the
language: singer.language entry to language: language.
In `@packages/core/src/search/interface.ts`:
- Around line 71-80: The loop only upserts current locales and leaves stale
documents in old locale indexes; update the code in the block using
collectLanguages, this.entityType, this.searchManager.getAdminIndex,
getDocument, index.addDocuments and this.searchManager.waitForTask to also
remove any documents for this entity.id that exist in locale-specific indexes no
longer present in collectLanguages(entity): enumerate relevant indexes for
this.entityType (or a predefined set of locale suffixes), for each index whose
language is not in the current languages call index.deleteDocument(entity.id)
(or the admin delete API) and await this.searchManager.waitForTask on the delete
task before/after the upserts so removed locales are cleaned up.
- Around line 89-102: Wrap the call to this.embeddingManager.embeddings.post
inside a try/catch so failures don't bubble up; on error (or if
embeddingResponse is missing) set embeddingAvailable = false and call
index.search without the vector and hybrid options (i.e., pass undefined for
vector and hybrid) to fall back to lexical search. Update the logic around
embeddingResponse/embeddingAvailable used when invoking index.search
(referencing embeddingManager.embeddings.post, embeddingResponse,
embeddingAvailable, and index.search) so a caught error results in a plain-text
search rather than throwing.
In `@scripts/import.ts`:
- Around line 166-178: The switch on data.type currently falls back to creating
a singer for unknown types; update the default branch in the switch (the block
around switch (data.type)) to NOT call importSinger for unsupported types like
"album", "series", etc.; instead log a clear warning including data.type and
file and skip processing (return/continue) so no bogus singer records are
created—use the existing importSinger/importArtist/importSong symbols to locate
the switch and ensure unknown types are safely ignored.
- Around line 181-194: The script currently logs per-file errors inside the
catch in main but always calls process.exit(0) in the finally, hiding failures;
update main and the file-level error handling so failures propagate: add a
boolean flag (e.g., importFailed) in the module scope or inside main, set
importFailed = true inside the per-file catch block where e is logged, remove
the unconditional process.exit(0) from the finally, and change the top-level
promise handlers so that main().catch(...) calls process.exit(1) and the finally
only logs completion (or calls process.exit(importFailed ? 1 : 0)); reference
the main() invocation, the per-file catch block, and the finally handler when
making these changes.
- Around line 103-108: The publishedAt value is being converted with new
Date(data.publishedAt ?? "").toISOString() inside the song creation payload,
which throws for empty/invalid dates; update the code around the
songService.create call to safely parse and validate data.publishedAt (e.g.,
create a Date object or use Date.parse and check isNaN/getTime) and only set the
publishedAt field when the parsed date is valid, otherwise leave publishedAt
undefined so invalid inputs don’t cause an exception during import.
---
Nitpick comments:
In `@apps/backend/src/handlers/catalog/artistRole/search.ts`:
- Around line 21-23: The request schema for the search handler accepts a raw q
string without bounds or normalization; update the z.object query schema in
search.ts (the query definition that contains q) to trim and enforce a max
length (e.g. z.string().trim().max(...).optional()) so incoming q is normalized
and bounded, and in the search handler use the normalized value (the
trimmed/validated q) before calling the search routine to avoid expensive
unbounded queries.
- Around line 6-24: Add an explicit response schema for artistRoleSearchHandler
by defining and using a DTO/validation schema (e.g.,
ArtistRoleSearchResponseSchema) that describes the 200 response shape (likely an
array of artist-role objects or a wrapper with items and metadata) and attach it
to the handler options under a response key (for example response: { 200:
ArtistRoleSearchResponseSchema }). Ensure the schema matches the objects
returned by artistRoleSearchService.search and update any import/exports to wire
ArtistRoleSearchResponseSchema into the handler options so client generation and
contract checks can validate the endpoint.
In `@packages/core/tests/unit/ArtistRoleService.test.ts`:
- Around line 81-86: The test currently calls artistRoleService.getDetails(999)
three times; capture the single returned promise (e.g., const promise =
artistRoleService.getDetails(999)) and reuse that promise for all three rejects
assertions (expect(promise).rejects.toThrow(AppError);
expect(promise).rejects.toThrow("error.artistRole.notfound");
expect(promise).rejects.toMatchObject({ code: "NOT_FOUND", statusCode: 404 });)
so the service is invoked only once and all matchers run against the same
rejected promise.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: d8cfef22-4166-4209-ac58-aec1c6ec4504
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (38)
apps/backend/src/handlers/auth/login.tsapps/backend/src/handlers/auth/signup.tsapps/backend/src/handlers/catalog/artistRole/create.tsapps/backend/src/handlers/catalog/artistRole/delete.tsapps/backend/src/handlers/catalog/artistRole/get.tsapps/backend/src/handlers/catalog/artistRole/index.tsapps/backend/src/handlers/catalog/artistRole/search.tsapps/backend/src/handlers/catalog/artistRole/update.tsapps/backend/src/handlers/index.tsapps/backend/src/index.tsapps/backend/src/middlewares/auth.tsapps/backend/tests/artistRole.test.tspackages/core/src/modules/auth/betterAuth.tspackages/core/src/modules/catalog/artistRole/container.tspackages/core/src/modules/catalog/artistRole/dto.tspackages/core/src/modules/catalog/artistRole/index.tspackages/core/src/modules/catalog/artistRole/repository.interface.tspackages/core/src/modules/catalog/artistRole/repository.tspackages/core/src/modules/catalog/artistRole/service.tspackages/core/src/modules/catalog/index.tspackages/core/src/outbox/processor.tspackages/core/src/outbox/queue.tspackages/core/src/search/catalog/artist.tspackages/core/src/search/catalog/artistRole.tspackages/core/src/search/catalog/index.tspackages/core/src/search/catalog/singer.tspackages/core/src/search/catalog/song.tspackages/core/src/search/config.tspackages/core/src/search/interface.tspackages/core/tests/integration/ArtistRoleRepository.test.tspackages/core/tests/unit/ArtistRoleService.test.tspackages/core/tests/unit/ArtistSearchService.test.tspackages/core/tests/unit/SingerSearchService.test.tspackages/db/prisma/models/core/artistRole.prismapackages/db/prisma/models/core/song.prismapackages/embedding/src/embedding.tsscripts/import.tsscripts/tsconfig.json
Comment on lines
+17
to
+62
| export const loginHandler = new Elysia().use(ip()).post( | ||
| "/session", | ||
| async ({ body, status, headers, cookie: { token: tokenCookie } }) => { | ||
| const { user, token } = await traceTask("auth.signInEmail", async () => { | ||
| return await auth.api.signInEmail({ | ||
| body: { | ||
| email: body.email, | ||
| password: body.password, | ||
| }, | ||
| headers: toBetterAuthHeaders(headers), | ||
| }); | ||
| }); | ||
|
|
||
| if (!token) { | ||
| throw new AppError("error.login.failed", "INTERNAL_SERVER_ERROR", 500, { | ||
| cause: "Better Auth responded with no token", | ||
| }); | ||
| } | ||
| if (!token) { | ||
| throw new AppError("error.login.failed", "INTERNAL_SERVER_ERROR", 500, { | ||
| cause: "Better Auth responded with no token", | ||
| }); | ||
| } | ||
|
|
||
| tokenCookie.value = token; | ||
| tokenCookie.httpOnly = true; | ||
| tokenCookie.maxAge = 90 * DAY; | ||
| tokenCookie.secure = true; | ||
| tokenCookie.sameSite = "lax"; | ||
| tokenCookie.value = token; | ||
| tokenCookie.httpOnly = true; | ||
| tokenCookie.maxAge = 90 * DAY; | ||
| tokenCookie.secure = true; | ||
| tokenCookie.sameSite = "lax"; | ||
|
|
||
| const userInfo = betterAuthToLoginUserInfoDto(user, token); | ||
| const response = toLoginResponse(userInfo); | ||
| const userInfo = betterAuthToLoginUserInfoDto(user, token); | ||
| const response = toLoginResponse(userInfo); | ||
|
|
||
| return status(200, response); | ||
| return status(200, response); | ||
| }, | ||
| { | ||
| body: LoginRequestSchema, | ||
| detail: { | ||
| summary: "User Login", | ||
| description: | ||
| "Authenticate an existing user with email and password credentials. Returns user info and sets an httpOnly authentication cookie.", | ||
| }, | ||
| { | ||
| body: LoginRequestSchema, | ||
| detail: { | ||
| summary: "User Login", | ||
| description: | ||
| "Authenticate an existing user with email and password credentials. Returns user info and sets an httpOnly authentication cookie.", | ||
| }, | ||
| response: { | ||
| 200: LoginResponseSchema, | ||
| 401: ErrorResponseSchema, | ||
| 422: ErrorResponseSchema, | ||
| 429: ErrorResponseSchema, | ||
| 500: ErrorResponseSchema, | ||
| }, | ||
| } | ||
| ); | ||
| response: { | ||
| 200: LoginResponseSchema, | ||
| 401: ErrorResponseSchema, | ||
| 422: ErrorResponseSchema, | ||
| 429: ErrorResponseSchema, | ||
| 500: ErrorResponseSchema, | ||
| }, | ||
| } | ||
| ); |
There was a problem hiding this comment.
Reintroduce throttling on the login endpoint.
POST /session is now unthrottled, which materially increases brute-force and credential-stuffing risk on a critical auth surface. Please restore per-IP/per-identifier rate limiting here (or wire an equivalent guaranteed control at the edge and document it in code).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/backend/src/handlers/auth/login.ts` around lines 17 - 62, The login
endpoint (loginHandler for POST "/session") currently lacks throttling and needs
per-IP and per-identifier rate limiting reinstated: add a rate-limit
check/middleware into the request pipeline for loginHandler (e.g., immediately
before the handler where ip() is used) that enforces a strict per-IP limit and a
per-identifier (email/username) limit, return appropriate 429 responses on
exceed, and document the control in the route detail; ensure the check uses the
incoming headers/body to key identifier-based limits and integrates with your
existing rate-limit provider or an in-memory/fallback store so the protection is
enforced at the application level if not guaranteed at the edge.
Comment on lines
+18
to
+60
| export const signupHandler = new Elysia().use(ip()).post( | ||
| "/user", | ||
| async ({ body, status, headers, cookie: { token: tokenCookie } }) => { | ||
| const { user, token } = await traceTask("auth.signUpEmail", async () => { | ||
| return await auth.api.signUpEmail({ | ||
| body: signupRequestToBetterAuth(body), | ||
| headers: toBetterAuthHeaders(headers), | ||
| }); | ||
| }); | ||
|
|
||
| if (!token) { | ||
| throw new AppError("error.signup.failed", "INTERNAL_SERVER_ERROR", 500, { | ||
| cause: "Better Auth responded with no token", | ||
| }); | ||
| } | ||
| if (!token) { | ||
| throw new AppError("error.signup.failed", "INTERNAL_SERVER_ERROR", 500, { | ||
| cause: "Better Auth responded with no token", | ||
| }); | ||
| } | ||
|
|
||
| tokenCookie.value = token; | ||
| tokenCookie.httpOnly = true; | ||
| tokenCookie.maxAge = 90 * DAY; | ||
| tokenCookie.secure = true; | ||
| tokenCookie.sameSite = "lax"; | ||
| tokenCookie.value = token; | ||
| tokenCookie.httpOnly = true; | ||
| tokenCookie.maxAge = 90 * DAY; | ||
| tokenCookie.secure = true; | ||
| tokenCookie.sameSite = "lax"; | ||
|
|
||
| const userInfo = betterAuthToSignupUserInfoDto(user, token); | ||
| const response = toSignUpResponse(userInfo); | ||
| const userInfo = betterAuthToSignupUserInfoDto(user, token); | ||
| const response = toSignUpResponse(userInfo); | ||
|
|
||
| return status(200, response); | ||
| return status(200, response); | ||
| }, | ||
| { | ||
| body: SignupRequestSchema, | ||
| detail: { | ||
| summary: "User Registration", | ||
| description: | ||
| "Register a new user account with email and password. Returns the created user info and sets an httpOnly authentication cookie.", | ||
| }, | ||
| { | ||
| body: SignupRequestSchema, | ||
| detail: { | ||
| summary: "User Registration", | ||
| description: | ||
| "Register a new user account with email and password. Returns the created user info and sets an httpOnly authentication cookie.", | ||
| }, | ||
| response: { | ||
| 200: SignupResponseSchema, | ||
| 400: ErrorResponseSchema, | ||
| 422: ErrorResponseSchema, | ||
| 429: ErrorResponseSchema, | ||
| 500: ErrorResponseSchema, | ||
| }, | ||
| } | ||
| ); | ||
| response: { | ||
| 200: SignupResponseSchema, | ||
| 400: ErrorResponseSchema, | ||
| 422: ErrorResponseSchema, | ||
| 429: ErrorResponseSchema, | ||
| 500: ErrorResponseSchema, | ||
| }, | ||
| } | ||
| ); |
There was a problem hiding this comment.
Keep anti-abuse throttling on signup.
POST /user no longer has explicit rate limiting, which weakens abuse protection for automated account creation. Please add route-level throttling back (or enforce a guaranteed equivalent upstream control).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/backend/src/handlers/auth/signup.ts` around lines 18 - 60, The signup
route lost rate-limiting—reintroduce route-level throttling on the POST "/user"
handler (signupHandler) by applying a rate-limit/throttle middleware (or
middleware factory) to the Elysia instance or directly to the post("/user")
route; configure it to key by IP (use the existing ip() middleware or headers),
set a conservative limit (e.g., small requests per minute and a short
ban/penalty window), and ensure the middleware returns appropriate 429 responses
and is included in the route options so automated signups are rate-limited while
legitimate users are unaffected.
Comment on lines
+28
to
+31
| response: { | ||
| 201: ArtistRoleResponseSchema, | ||
| 401: ErrorResponseSchema, | ||
| }, |
There was a problem hiding this comment.
Document 400 validation errors in the create route response map.
This route validates body, so bad payloads can return 400 at runtime. Adding 400 here keeps the OpenAPI contract accurate.
Suggested patch
response: {
+ 400: ErrorResponseSchema,
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| response: { | |
| 201: ArtistRoleResponseSchema, | |
| 401: ErrorResponseSchema, | |
| }, | |
| response: { | |
| 201: ArtistRoleResponseSchema, | |
| 400: ErrorResponseSchema, | |
| 401: ErrorResponseSchema, | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/backend/src/handlers/catalog/artistRole/create.ts` around lines 28 - 31,
The response map for the create route in
apps/backend/src/handlers/catalog/artistRole/create.ts is missing a 400 entry
for validation errors; update the response object (the response map used by the
create handler) to include 400: ErrorResponseSchema alongside 201:
ArtistRoleResponseSchema and 401: ErrorResponseSchema so that body validation
failures are documented in the OpenAPI contract.
Comment on lines
+56
to
+60
| logger: { | ||
| log: (level, message, ...args) => { | ||
| appLogger[level](message, { | ||
| metadata: args, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Read-only check for existing redaction/sanitization controls in logger/auth paths.
rg -n -C2 --iglob '*.ts' 'redact|sanitize|mask|password|token|authorization|cookie|set-cookie'
rg -n -C2 --iglob '*.ts' 'logger:\s*\{|appLogger\[level\]|metadata:\s*args'Repository: project-cvsa/cvsa
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Inspect the exact lines in betterAuth.ts around 56-60 (and nearby)
sed -n '40,90p' packages/core/src/modules/auth/betterAuth.ts
# 2) Inspect pinoSetup.ts where appLogger is defined/initialized and how redact is used
sed -n '1,200p' packages/logger/src/pinoSetup.ts
# 3) Find where appLogger is exported/created, and any custom redaction/formatters
rg -n "export (const|let|function) appLogger|appLogger\s*=" packages/logger/src -S
rg -n "redact\s*:" packages/logger/src/pinoSetup.ts packages/logger/src -S
rg -n "metadata:\s*args|metadata:\s*\{|\bmetadata\b" packages/core/src/modules/auth/betterAuth.ts -S
# 4) Quick check for any other logger wrapper behavior that may already sanitize
rg -n "sanitize|redact|mask|allowlist" packages/logger/src packages/core/src/modules/auth -SRepository: project-cvsa/cvsa
Length of output: 1502
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# betterAuth.ts context around the logger block
sed -n '1,140p' packages/core/src/modules/auth/betterAuth.ts
# appLogger implementation and how metadata is handled
sed -n '1,240p' packages/logger/src/appLogger.ts
# Find all references to "metadata" usage in the logger package
rg -n "metadata" packages/logger/src -S
# Find createApplicationLogger definition details
rg -n "createApplicationLogger" packages/logger/src/appLogger.ts packages/logger/src -SRepository: project-cvsa/cvsa
Length of output: 5575
Do not forward raw auth logger arguments to log/OTEL metadata.
In packages/core/src/modules/auth/betterAuth.ts, the better-auth logger passes ...args straight into { metadata: args } with no sanitization/allowlist. packages/logger/src/appLogger.ts forwards those same attrs to both Pino and OpenTelemetry emit() attributes, so any sensitive auth data in args can leak outside Pino redaction (and Pino’s redaction only targets keys like authorization, cookie, password, token).
- Sanitize/allowlist
metadatabefore callingappLogger[...](and ensure the OTEL path doesn’t receive rawargs).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/modules/auth/betterAuth.ts` around lines 56 - 60, The
better-auth logger's log implementation forwards raw variadic args into
appLogger[level] as metadata (logger.log -> appLogger[level] with { metadata:
args }), which can leak sensitive auth fields to Pino/OTEL; sanitize or
allowlist the metadata before passing it on: inspect the incoming ...args in
logger.log, strip or redact sensitive keys (authorization, cookie, password,
token, etc.) or build an explicit allowlist of safe fields, then call
appLogger[level] with { metadata: sanitizedMetadata } (and ensure any OTEL emit
path receives only the sanitized/allowlisted attributes rather than the raw
args).
| import type { EmbeddingApp } from "@cvsa/embedding"; | ||
| import { outboxService } from "../../outbox/container"; | ||
|
|
||
| const embeddingManager = treaty<EmbeddingApp>("localhost:14900"); |
There was a problem hiding this comment.
Avoid hardcoded embedding service endpoint.
Line 9 hardcodes localhost:14900, which is environment-specific and brittle for deployed environments. Resolve from config/env and fail fast if missing.
Proposed fix
-const embeddingManager = treaty<EmbeddingApp>("localhost:14900");
+const embeddingServiceUrl = process.env.EMBEDDING_SERVICE_URL;
+if (!embeddingServiceUrl) {
+ throw new Error("EMBEDDING_SERVICE_URL is not configured");
+}
+const embeddingManager = treaty<EmbeddingApp>(embeddingServiceUrl);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const embeddingManager = treaty<EmbeddingApp>("localhost:14900"); | |
| const embeddingServiceUrl = process.env.EMBEDDING_SERVICE_URL; | |
| if (!embeddingServiceUrl) { | |
| throw new Error("EMBEDDING_SERVICE_URL is not configured"); | |
| } | |
| const embeddingManager = treaty<EmbeddingApp>(embeddingServiceUrl); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/modules/catalog/artistRole/container.ts` at line 9, The
embedding service address is hardcoded in the creation of embeddingManager via
treaty<EmbeddingApp>("localhost:14900"); change this to read the endpoint from
configuration/environment (e.g., process.env.EMBEDDING_SERVICE_URL or your app
config accessor) and validate it before use; if the value is missing or invalid,
throw an error or exit early so the process fails fast. Update the code that
constructs embeddingManager to use the resolved variable and include a clear
error message referencing embeddingManager/treaty<EmbeddingApp> when aborting.
Comment on lines
+71
to
+80
| const languages = this.collectLanguages(entity); | ||
| for (const language of languages) { | ||
| const indexUid = `${this.entityType}_${language}`; | ||
| const index = await this.searchManager.getAdminIndex<TIndex>(indexUid); | ||
| const document = await this.getDocument(entity, language); | ||
| const task = await index.addDocuments([document], { | ||
| primaryKey: "id", | ||
| }); | ||
| await this.searchManager.waitForTask(task.taskUid); | ||
| } |
There was a problem hiding this comment.
Clean up documents from locales that were removed.
This loop only upserts the entity's current locales. If a translation is deleted or the base language changes, the old ${this.entityType}_${language} document is left behind and keeps showing up in search until a full reindex.
Suggested fix
const languages = this.collectLanguages(entity);
+ const desiredIndexes = new Set(
+ languages.map((language) => `${this.entityType}_${language}`)
+ );
+ const existingIndexes =
+ await this.searchManager.getLocalizedIndexesOfEntity(this.entityType);
+
+ for (const indexName of existingIndexes) {
+ if (!desiredIndexes.has(indexName)) {
+ const adminIndex = await this.searchManager.getAdminIndex(indexName);
+ const task = await adminIndex.deleteDocument(id);
+ await this.searchManager.waitForTask(task.taskUid);
+ }
+ }
+
for (const language of languages) {
const indexUid = `${this.entityType}_${language}`;
const index = await this.searchManager.getAdminIndex<TIndex>(indexUid);
const document = await this.getDocument(entity, language);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/search/interface.ts` around lines 71 - 80, The loop only
upserts current locales and leaves stale documents in old locale indexes; update
the code in the block using collectLanguages, this.entityType,
this.searchManager.getAdminIndex, getDocument, index.addDocuments and
this.searchManager.waitForTask to also remove any documents for this entity.id
that exist in locale-specific indexes no longer present in
collectLanguages(entity): enumerate relevant indexes for this.entityType (or a
predefined set of locale suffixes), for each index whose language is not in the
current languages call index.deleteDocument(entity.id) (or the admin delete API)
and await this.searchManager.waitForTask on the delete task before/after the
upserts so removed locales are cleaned up.
Comment on lines
+89
to
+102
| const embeddingResponse = await this.embeddingManager.embeddings.post({ | ||
| texts: [query], | ||
| }); | ||
| const embeddingAvailable = (embeddingResponse?.data?.embeddings[0]?.length ?? 0) > 0; | ||
|
|
||
| return index.search(query, { | ||
| vector: embeddingResponse?.data?.embeddings[0], | ||
| hybrid: embeddingAvailable | ||
| ? { | ||
| embedder: "potion-multilingual-128M", | ||
| semanticRatio: 0.25, | ||
| } | ||
| : undefined, | ||
| showRankingScore: true, |
There was a problem hiding this comment.
Fall back to lexical search when embedding lookup fails.
An exception from embeddings.post() still takes down the whole search request. Since this method already supports a no-vector path, it should catch embedding failures and continue with plain text search instead of returning a 5xx for every transient embedding outage.
Suggested fix
const index = await this.searchManager.getSearchIndex(`${this.entityType}_${language}`);
- const embeddingResponse = await this.embeddingManager.embeddings.post({
- texts: [query],
- });
- const embeddingAvailable = (embeddingResponse?.data?.embeddings[0]?.length ?? 0) > 0;
+ let embedding: number[] | undefined;
+ try {
+ const embeddingResponse = await this.embeddingManager.embeddings.post({
+ texts: [query],
+ });
+ embedding = embeddingResponse?.data?.embeddings[0];
+ } catch (error) {
+ appLogger.warn(
+ { error, entityType: this.entityType, language },
+ "Embedding lookup failed; falling back to lexical search"
+ );
+ }
+ const embeddingAvailable = (embedding?.length ?? 0) > 0;
return index.search(query, {
- vector: embeddingResponse?.data?.embeddings[0],
+ vector: embedding,
hybrid: embeddingAvailable
? {
embedder: "potion-multilingual-128M",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const embeddingResponse = await this.embeddingManager.embeddings.post({ | |
| texts: [query], | |
| }); | |
| const embeddingAvailable = (embeddingResponse?.data?.embeddings[0]?.length ?? 0) > 0; | |
| return index.search(query, { | |
| vector: embeddingResponse?.data?.embeddings[0], | |
| hybrid: embeddingAvailable | |
| ? { | |
| embedder: "potion-multilingual-128M", | |
| semanticRatio: 0.25, | |
| } | |
| : undefined, | |
| showRankingScore: true, | |
| const index = await this.searchManager.getSearchIndex(`${this.entityType}_${language}`); | |
| let embedding: number[] | undefined; | |
| try { | |
| const embeddingResponse = await this.embeddingManager.embeddings.post({ | |
| texts: [query], | |
| }); | |
| embedding = embeddingResponse?.data?.embeddings[0]; | |
| } catch (error) { | |
| appLogger.warn( | |
| { error, entityType: this.entityType, language }, | |
| "Embedding lookup failed; falling back to lexical search" | |
| ); | |
| } | |
| const embeddingAvailable = (embedding?.length ?? 0) > 0; | |
| return index.search(query, { | |
| vector: embedding, | |
| hybrid: embeddingAvailable | |
| ? { | |
| embedder: "potion-multilingual-128M", | |
| semanticRatio: 0.25, | |
| } | |
| : undefined, | |
| showRankingScore: true, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/search/interface.ts` around lines 89 - 102, Wrap the call
to this.embeddingManager.embeddings.post inside a try/catch so failures don't
bubble up; on error (or if embeddingResponse is missing) set embeddingAvailable
= false and call index.search without the vector and hybrid options (i.e., pass
undefined for vector and hybrid) to fall back to lexical search. Update the
logic around embeddingResponse/embeddingAvailable used when invoking
index.search (referencing embeddingManager.embeddings.post, embeddingResponse,
embeddingAvailable, and index.search) so a caught error results in a plain-text
search rather than throwing.
Comment on lines
+103
to
+108
| const song = await songService.create({ | ||
| name: data.name, | ||
| type: data.songType ? (songTypeMap[data.songType] as never) : undefined, | ||
| description: data.description, | ||
| publishedAt: new Date(data.publishedAt ?? "").toISOString(), | ||
| performances: performances.length ? performances : undefined, |
There was a problem hiding this comment.
Handle optional/invalid publishedAt safely before toISOString().
Line 107 can throw on empty/invalid input (Invalid Date), which aborts that file’s song import.
Suggested fix
+ const parsedPublishedAt = data.publishedAt ? new Date(data.publishedAt) : undefined;
+ const publishedAt =
+ parsedPublishedAt && !Number.isNaN(parsedPublishedAt.getTime())
+ ? parsedPublishedAt.toISOString()
+ : undefined;
+
const song = await songService.create({
name: data.name,
type: data.songType ? (songTypeMap[data.songType] as never) : undefined,
description: data.description,
- publishedAt: new Date(data.publishedAt ?? "").toISOString(),
+ publishedAt,
performances: performances.length ? performances : undefined,
creations: creations.length ? creations : undefined,
lyrics: lyrics.length ? lyrics : undefined,
});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/import.ts` around lines 103 - 108, The publishedAt value is being
converted with new Date(data.publishedAt ?? "").toISOString() inside the song
creation payload, which throws for empty/invalid dates; update the code around
the songService.create call to safely parse and validate data.publishedAt (e.g.,
create a Date object or use Date.parse and check isNaN/getTime) and only set the
publishedAt field when the parsed date is valid, otherwise leave publishedAt
undefined so invalid inputs don’t cause an exception during import.
Comment on lines
+166
to
+178
| switch (data.type) { | ||
| case "singer": | ||
| await importSinger(data); | ||
| break; | ||
| case "artist": | ||
| await importArtist(data); | ||
| break; | ||
| case "song": | ||
| await importSong(data); | ||
| break; | ||
| default: | ||
| await importSinger(data); | ||
| console.warn(`unkown type ${data.type} for file ${file}`); |
There was a problem hiding this comment.
Don’t import unknown types as singers.
Lines 176-178 create singer records for unsupported types (album, series, others), which can pollute catalog data.
Suggested fix
switch (data.type) {
case "singer":
await importSinger(data);
break;
case "artist":
await importArtist(data);
break;
case "song":
await importSong(data);
break;
+ case "album":
+ case "series":
+ case "others":
+ console.warn(`unsupported type ${data.type} for file ${file}`);
+ break;
default:
- await importSinger(data);
- console.warn(`unkown type ${data.type} for file ${file}`);
+ console.warn(`unknown type ${data.type} for file ${file}`);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| switch (data.type) { | |
| case "singer": | |
| await importSinger(data); | |
| break; | |
| case "artist": | |
| await importArtist(data); | |
| break; | |
| case "song": | |
| await importSong(data); | |
| break; | |
| default: | |
| await importSinger(data); | |
| console.warn(`unkown type ${data.type} for file ${file}`); | |
| switch (data.type) { | |
| case "singer": | |
| await importSinger(data); | |
| break; | |
| case "artist": | |
| await importArtist(data); | |
| break; | |
| case "song": | |
| await importSong(data); | |
| break; | |
| case "album": | |
| case "series": | |
| case "others": | |
| console.warn(`unsupported type ${data.type} for file ${file}`); | |
| break; | |
| default: | |
| console.warn(`unknown type ${data.type} for file ${file}`); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/import.ts` around lines 166 - 178, The switch on data.type currently
falls back to creating a singer for unknown types; update the default branch in
the switch (the block around switch (data.type)) to NOT call importSinger for
unsupported types like "album", "series", etc.; instead log a clear warning
including data.type and file and skip processing (return/continue) so no bogus
singer records are created—use the existing importSinger/importArtist/importSong
symbols to locate the switch and ensure unknown types are safely ignored.
Comment on lines
+181
to
+194
| catch (e) { | ||
| console.error("error at: ", file); | ||
| console.error(e); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| main().catch((err) => { | ||
| console.error("fatal:", err); | ||
| process.exit(1); | ||
| }).finally(() => { | ||
| console.log("completed"); | ||
| process.exit(0); | ||
| }) |
There was a problem hiding this comment.
Propagate import failures via non-zero exit status.
Current flow logs per-file errors but still ends with success (process.exit(0)), which hides failed imports from CI/ops.
Suggested fix
async function main() {
+ let hadErrors = false;
...
for (const file of files) {
try {
...
}
catch (e) {
+ hadErrors = true;
console.error("error at: ", file);
console.error(e);
}
}
+
+ if (hadErrors) {
+ throw new Error("Import completed with errors");
+ }
}
-main().catch((err) => {
- console.error("fatal:", err);
- process.exit(1);
-}).finally(() => {
- console.log("completed");
- process.exit(0);
-})
+main()
+ .then(() => {
+ console.log("completed");
+ process.exit(0);
+ })
+ .catch((err) => {
+ console.error("fatal:", err);
+ process.exit(1);
+ });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/import.ts` around lines 181 - 194, The script currently logs per-file
errors inside the catch in main but always calls process.exit(0) in the finally,
hiding failures; update main and the file-level error handling so failures
propagate: add a boolean flag (e.g., importFailed) in the module scope or inside
main, set importFailed = true inside the per-file catch block where e is logged,
remove the unconditional process.exit(0) from the finally, and change the
top-level promise handlers so that main().catch(...) calls process.exit(1) and
the finally only logs completion (or calls process.exit(importFailed ? 1 : 0));
reference the main() invocation, the per-file catch block, and the finally
handler when making these changes.
alikia2x
force-pushed
the
feat/artist-role
branch
from
a568ac7 to
9f804ce
Compare
alikia2x
force-pushed
the
feat/artist-role
branch
from
9f804ce to
a96af0d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes Made in This PR
Core Feature Implementation
Added artistRole CRUD API handlers: Implemented five new Elysia route handlers for artist roles:
POST /artist-role- Create new artist role with authenticationGET /artist-role/:id- Retrieve artist role detailsPATCH /artist-role/:id- Update artist role with authenticationDELETE /artist-role/:id- Soft delete artist role with authenticationGET /artist-roles- Search artist roles with optional query parameterImplemented artistRole service layer:
ArtistRoleServiceclass withgetDetails(),create(),update(), anddelete()methodsArtistRoleRepositoryclass handling Prisma queries with transaction supportArtistRoleSearchServicefor full-text search with embedding supportAdded database schema updates:
ArtistRolePrisma model withlanguagefield (default: "zh")SongTypeenum values:RETUNE,CONTRAFACTUM,TRANSLYRICSSearch & Indexing Enhancements
Refactored search service architecture:
ISearchServicebase class to support localized entities with per-language indexingsync()andsearch()methods in base class instead of abstract methodsAdded search service implementations:
ArtistRoleSearchServicewith embedding support using "potion-multilingual-128M"SongSearchService,ArtistSearchService, andSingerSearchServiceto use new architectureAdded search configuration:
Event & Outbox Integration
artistRoleSearchServiceinto outbox processor for event handlingartistRole.created,artistRole.updated, andartistRole.deletedeventsAuthentication & API Changes
Removed rate limiting middleware:
loginHandlerto remove Elysia rate limiting (keep onlyip()middleware)signupHandlerto remove rate limiting middlewareAdded auth logger integration: Wired
appLoggerinto betterAuth configuration for loggingTesting & Data Import
Added comprehensive test coverage:
artistRole.test.ts)ArtistRoleRepositorywith full CRUD verificationArtistRoleServicewith mocked dependenciesImplemented data import script (
scripts/import.ts):DTOs & Type Definitions
CreateArtistRoleRequestSchemaandUpdateArtistRoleRequestSchemawith localization supportArtistRoleResponseSchemaandArtistRoleDetailsResponseSchemafor API responsesModule & Export Organization
Created artistRole module structure:
packages/core/src/modules/catalog/artistRole/index.tsUpdated handler registration: Registered
artistRoleHandlerin backend app beforesingerHandlerSupporting Changes
EmbeddingManager.getEmbeddingscripts/tsconfig.jsonwith proper path aliases and compilation settingsappLoggerimport to auth middleware