Skip to content

Conversation

@DaxServer
Copy link
Owner

This commit introduces a new ReplaceOperationService that provides functionality to perform
replace operations on a column in a project table. The service supports case-sensitive and
whole-word replacement, and it calculates the number of affected rows before executing the
update.

The changes include:

  • Implement the ReplaceOperationService class with methods to build the appropriate replace
    expression, count the affected rows, and execute the update.
  • Add new API endpoint /projects/:projectId/replace that allows users to perform a replace
    operation on a project table.
  • Introduce new request and response schemas for the replace operation endpoint.

These changes enable users to easily replace values in project table columns, which is a
common requirement for data manipulation tasks.

This commit introduces a new `ReplaceOperationService` that provides functionality to perform
replace operations on a column in a project table. The service supports case-sensitive and
whole-word replacement, and it calculates the number of affected rows before executing the
update.

The changes include:

- Implement the `ReplaceOperationService` class with methods to build the appropriate replace
  expression, count the affected rows, and execute the update.
- Add new API endpoint `/projects/:projectId/replace` that allows users to perform a replace
  operation on a project table.
- Introduce new request and response schemas for the replace operation endpoint.

These changes enable users to easily replace values in project table columns, which is a
common requirement for data manipulation tasks.
@coderabbitai
Copy link

coderabbitai bot commented Sep 28, 2025

📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Added a POST endpoint to perform bulk find-and-replace on a project’s table column, with options for case sensitivity and whole-word matching. Returns the number of updated rows and provides clear validation and error responses.
  • Tests

    • Introduced comprehensive integration and service tests covering basic replacements, case sensitivity, whole-word behavior, empty replacements, special characters, and error scenarios (invalid columns/tables and missing fields).

Walkthrough

Adds a new POST /:projectId/replace API that validates input, delegates replace logic to a new ReplaceOperationService which builds and runs parameterized DuckDB updates (with case/whole-word options), and returns affected row counts; includes schemas and extensive service+API tests.

Changes

Cohort / File(s) Summary of changes
API route
backend/src/api/project/index.ts
New POST /:projectId/replace route: validates ReplaceOperationSchema, verifies column existence, calls ReplaceOperationService.performReplace, returns affected rows or errors (400/500).
Schemas
backend/src/api/project/schemas.ts
Added and exported ReplaceOperationSchema (fields: column, find, replace, caseSensitive, wholeWord with defaults/validation).
Service implementation
backend/src/services/replace-operation.service.ts
New ReplaceOperationService with performReplace, countAffectedRows, buildParameterizedUpdateQuery, escapeRegex; supports partial/whole-word, case-sensitive/insensitive replacements using DuckDB functions and parameterized queries. Exports ReplaceOperationParams and ReplaceOperationResult types.
API integration tests
backend/tests/api/project/project.replace.test.ts
New integration tests for the replace endpoint covering success cases (case/whole-word/empty replacement/special chars), validation errors, non-existent column (400), and data verification after updates.
Service unit tests
backend/tests/services/replace-operation.service.test.ts
New tests for ReplaceOperationService.performReplace covering matching variants, edge cases (no-match), special characters, and error conditions (missing table/column).

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant API as ProjectAPI
  participant Svc as ReplaceOperationService
  participant DB as DuckDB

  Client->>API: POST /:projectId/replace { column, find, replace, caseSensitive, wholeWord }
  alt validation fails
    API-->>Client: 400 / 422 (validation or missing column)
  else valid
    API->>Svc: performReplace(params)
    Svc->>DB: COUNT ... (match query)  — parameterized
    DB-->>Svc: affectedCount
    alt affectedCount > 0
      Svc->>DB: UPDATE ... (regexp_replace / replace) — parameterized
      DB-->>Svc: updateResult
    end
    Svc-->>API: affectedCount
    API-->>Client: 200 { message, affectedRows }
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title uses conventional commit syntax to clearly indicate a new feature and accurately summarizes the main addition of the ReplaceOperationService without extraneous details, making it concise and directly related to the changeset.
Description Check ✅ Passed The description outlines the key additions including the ReplaceOperationService implementation, the new API endpoint, and associated request and response schemas, directly reflecting the changes in the code without deviating off-topic.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch replace-operation-service

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (5)
backend/src/api/project/index.ts (1)

559-571: Return a consistent envelope or set status explicitly.

Other endpoints return { data: ... }. If you intend to keep a bare payload, at least set status(200, result) for consistency across handlers.

-        return result
+        return status(200, result)
backend/src/api/project/schemas.ts (1)

53-72: Tighten column input to valid identifiers (defense-in-depth).

Since column is later interpolated as an identifier, constrain it to existing identifier characters to avoid odd edge cases from imported headers.

 export const ReplaceOperationSchema = t.Object({
-  column: t.String({
-    minLength: 1,
-    error: 'Column name is required and must be at least 1 character long',
-  }),
+  column: t.String({
+    minLength: 1,
+    pattern: '^[A-Za-z_][A-Za-z0-9_\\s-]{0,127}$',
+    error:
+      'Column name is required and must start with a letter/underscore and contain only letters, digits, spaces, dashes, or underscores (<=128 chars)',
+  }),

Note: adjust the pattern to match your identifier policy if spaces/dashes are not allowed in your tables.

backend/src/services/replace-operation.service.ts (3)

52-75: Deprecate manual string interpolation in expression builder.

With the parameterized variant above, you can remove or keep this only for legacy callers. Prefer the parameterized helper to avoid escaping pitfalls.


80-107: Avoid passing empty regex flags.

For the whole-word + case-sensitive path, you currently pass '' to regexp_matches. Prefer omitting the flags arg entirely to prevent dialect quirks.

-      const flags = caseSensitive ? '' : 'i'
-      const pattern = `\\b${this.escapeRegex(find)}\\b`
-      countQuery = `SELECT COUNT(*) as count FROM "${table}" WHERE "${column}" IS NOT NULL AND regexp_matches("${column}", '${pattern}', '${flags}')`
+      const pattern = `\\b${this.escapeRegex(find)}\\b`
+      countQuery = caseSensitive
+        ? `SELECT COUNT(*) as count FROM "${table}" WHERE "${column}" IS NOT NULL AND regexp_matches("${column}", '${pattern}')`
+        : `SELECT COUNT(*) as count FROM "${table}" WHERE "${column}" IS NOT NULL AND regexp_matches("${column}", '${pattern}', 'i')`

112-121: Simplify escaping once parameters are used.

If you adopt bound parameters, escapeSql becomes unnecessary for values. Keep escapeRegex for building patterns, but drop the trailing .replace(/'/g, "''").

-  private escapeRegex(str: string): string {
-    return str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&').replace(/'/g, "''")
-  }
+  private escapeRegex(str: string): string {
+    return str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')
+  }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 47d67c3 and 393fdea.

📒 Files selected for processing (5)
  • backend/src/api/project/index.ts (2 hunks)
  • backend/src/api/project/schemas.ts (1 hunks)
  • backend/src/services/replace-operation.service.ts (1 hunks)
  • backend/tests/api/project/project.replace.test.ts (1 hunks)
  • backend/tests/services/replace-operation.service.test.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
backend/tests/services/replace-operation.service.test.ts (2)
backend/src/services/replace-operation.service.ts (1)
  • ReplaceOperationService (17-122)
backend/src/plugins/database.ts (3)
  • initializeDb (8-48)
  • getDb (50-55)
  • closeDb (57-63)
backend/src/api/project/index.ts (4)
backend/src/types/error-handler.ts (1)
  • ApiErrorHandler (6-211)
backend/src/services/replace-operation.service.ts (1)
  • ReplaceOperationService (17-122)
backend/src/api/project/schemas.ts (4)
  • ReplaceOperationSchema (54-72)
  • ReplaceOperationSchema (73-73)
  • ReplaceOperationResponseSchema (75-78)
  • ReplaceOperationResponseSchema (79-79)
backend/src/types/error-schemas.ts (2)
  • ApiErrors (20-26)
  • ApiErrors (27-27)
backend/tests/api/project/project.replace.test.ts (1)
backend/src/plugins/database.ts (2)
  • initializeDb (8-48)
  • closeDb (57-63)
🔇 Additional comments (8)
backend/src/api/project/index.ts (3)

7-9: LGTM: exports wired for replace endpoint.

Importing ReplaceOperationResponseSchema and ReplaceOperationSchema here looks correct.


13-13: LGTM: service import.

ReplaceOperationService import aligns with its usage below.


583-596: Schema OK; verify 404 presence rationale.

The route can now return 404 after adding the table-exists check; keeping 404 in the response map remains appropriate.

backend/tests/services/replace-operation.service.test.ts (2)

127-145: Solid coverage for special chars.

Good addition of a row to exercise '@' handling and verifying affected count.


170-195: Direct service error paths covered.

Non-existent column/table cases correctly assert throws.

backend/src/api/project/schemas.ts (1)

75-79: Response schema looks right.

Matches the service response shape.

backend/tests/api/project/project.replace.test.ts (2)

244-263: Good validation assertion.

Accurately checks 400 structure for unknown column.


72-104: Nice end-to-end verification.

Validates both response and underlying data mutation.

Comment on lines +544 to +557
// Check if column exists
const columnExistsReader = await db().runAndReadAll(
'SELECT 1 FROM information_schema.columns WHERE table_name = ? AND column_name = ?',
[table, column],
)

if (columnExistsReader.getRows().length === 0) {
return status(
400,
ApiErrorHandler.validationErrorWithData('Column not found', [
`Column '${column}' does not exist in table '${table}'`,
]),
)
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Add explicit table existence check before column validation.

If the project table hasn’t been created yet, this returns a misleading “Column not found” (400). Check table existence first and return 404 to match the declared response types.

       const table = `project_${projectId}`

+      // Ensure project table exists
+      const tableExistsReader = await db().runAndReadAll(
+        'SELECT 1 FROM duckdb_tables() WHERE table_name = ?',
+        [table],
+      )
+      if (tableExistsReader.getRows().length === 0) {
+        return status(404, ApiErrorHandler.notFoundErrorWithData('Project table', table))
+      }
+
       // Check if column exists
-      const columnExistsReader = await db().runAndReadAll(
-        'SELECT 1 FROM information_schema.columns WHERE table_name = ? AND column_name = ?',
-        [table, column],
-      )
+      const columnExistsReader = await db().runAndReadAll(
+        `SELECT 1
+         FROM duckdb_columns()
+         WHERE table_name = ? AND column_name = ?`,
+        [table, column],
+      )
📝 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
// Check if column exists
const columnExistsReader = await db().runAndReadAll(
'SELECT 1 FROM information_schema.columns WHERE table_name = ? AND column_name = ?',
[table, column],
)
if (columnExistsReader.getRows().length === 0) {
return status(
400,
ApiErrorHandler.validationErrorWithData('Column not found', [
`Column '${column}' does not exist in table '${table}'`,
]),
)
}
const table = `project_${projectId}`
// Ensure project table exists
const tableExistsReader = await db().runAndReadAll(
'SELECT 1 FROM duckdb_tables() WHERE table_name = ?',
[table],
)
if (tableExistsReader.getRows().length === 0) {
return status(404, ApiErrorHandler.notFoundErrorWithData('Project table', table))
}
// Check if column exists
const columnExistsReader = await db().runAndReadAll(
`SELECT 1
FROM duckdb_columns()
WHERE table_name = ? AND column_name = ?`,
[table, column],
)
if (columnExistsReader.getRows().length === 0) {
return status(
400,
ApiErrorHandler.validationErrorWithData('Column not found', [
`Column '${column}' does not exist in table '${table}'`,
]),
)
}
🤖 Prompt for AI Agents
In backend/src/api/project/index.ts around lines 544 to 557, add an explicit
check that the target table exists before checking for the column: first query
information_schema.tables (e.g. SELECT 1 FROM information_schema.tables WHERE
table_name = ?) with the table name and if no rows returned return status(404,
ApiErrorHandler.notFound('Table not found', [`Table '${table}' does not
exist`])); only then run the existing information_schema.columns query to
validate the column and keep the existing 400 Column not found response if the
column is missing.

Comment on lines 81 to 83
expect(result.message).toBe("Successfully replaced 'John' with 'Jonathan' in column 'name'")
expect(result.affectedRows).toBe(2) // John Doe and John Johnson
})
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fix misleading test comment.

“John Johnson” isn’t in TEST_DATA; the second match comes from “Bob Johnson”.

-      expect(result.affectedRows).toBe(2) // John Doe and John Johnson
+      expect(result.affectedRows).toBe(2) // John Doe and Bob Johnson
📝 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
expect(result.message).toBe("Successfully replaced 'John' with 'Jonathan' in column 'name'")
expect(result.affectedRows).toBe(2) // John Doe and John Johnson
})
expect(result.message).toBe("Successfully replaced 'John' with 'Jonathan' in column 'name'")
expect(result.affectedRows).toBe(2) // John Doe and Bob Johnson
})
🤖 Prompt for AI Agents
In backend/tests/services/replace-operation.service.test.ts around lines 81 to
83, the inline test comment incorrectly states the two matched rows are "John
Doe and John Johnson" even though TEST_DATA contains "Bob Johnson"; update the
comment to accurately read "John Doe and Bob Johnson" (or similar phrasing) so
the comment matches the actual test data and expectation.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (2)
backend/src/services/replace-operation.service.ts (1)

57-63: Whole‑word semantics are ASCII‑centric; confirm expected behavior.

\b uses [A-Za-z0-9_] word chars. Names with non‑ASCII letters may not be matched as “whole words.” If you need Unicode semantics, call that out in API docs or implement a Unicode‑aware strategy.

backend/src/api/project/index.ts (1)

558-582: Minor: Propagate sanitized identifiers into the service (defense in depth).

Even with the route checks, the service builds SQL with identifiers. Prefer passing only validated table/column, or add server-side allowlisting/quoting in the service (see service comment).

Would you like me to push a follow-up PR wiring the quoteIdent helper into the service and adding identifier-fuzz tests?

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 393fdea and 705814d.

📒 Files selected for processing (5)
  • backend/src/api/project/index.ts (2 hunks)
  • backend/src/api/project/schemas.ts (1 hunks)
  • backend/src/services/replace-operation.service.ts (1 hunks)
  • backend/tests/api/project/project.replace.test.ts (1 hunks)
  • backend/tests/services/replace-operation.service.test.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • backend/tests/api/project/project.replace.test.ts
  • backend/src/api/project/schemas.ts
🧰 Additional context used
🧬 Code graph analysis (2)
backend/tests/services/replace-operation.service.test.ts (2)
backend/src/services/replace-operation.service.ts (1)
  • ReplaceOperationService (12-157)
backend/src/plugins/database.ts (3)
  • initializeDb (8-48)
  • getDb (50-55)
  • closeDb (57-63)
backend/src/api/project/index.ts (4)
backend/src/types/error-handler.ts (1)
  • ApiErrorHandler (6-211)
backend/src/services/replace-operation.service.ts (1)
  • ReplaceOperationService (12-157)
backend/src/api/project/schemas.ts (1)
  • ReplaceOperationSchema (54-72)
backend/src/types/error-schemas.ts (2)
  • ApiErrors (20-26)
  • ApiErrors (27-27)
🔇 Additional comments (2)
backend/tests/services/replace-operation.service.test.ts (1)

80-81: Fix misleading inline comment.

It’s “Bob Johnson,” not “John Johnson.”

Apply the change included in the diff above.

backend/src/api/project/index.ts (1)

543-557: Return 404 if table doesn’t exist; check table before column.

Current logic returns 400 “Column not found” when the project table is missing. Check table existence first (404), then validate the column (400).

Apply this diff:

   const table = `project_${projectId}`

-  // Check if column exists
-  const columnExistsReader = await db().runAndReadAll(
-    'SELECT 1 FROM information_schema.columns WHERE table_name = ? AND column_name = ?',
-    [table, column],
-  )
+  // Ensure table exists
+  const tableExistsReader = await db().runAndReadAll(
+    'SELECT 1 FROM duckdb_tables() WHERE table_name = ?',
+    [table],
+  )
+  if (tableExistsReader.getRows().length === 0) {
+    return status(404, ApiErrorHandler.notFoundErrorWithData('Project table', table))
+  }
+
+  // Check if column exists
+  const columnExistsReader = await db().runAndReadAll(
+    `SELECT 1
+     FROM duckdb_columns()
+     WHERE table_name = ? AND column_name = ?`,
+    [table, column],
+  )

Comment on lines +59 to +71
const replaceFlags = caseSensitive ? 'g' : 'gi'
const matchFlags = caseSensitive ? '' : 'i'
const pattern = `\\b${this.escapeRegex(find)}\\b`
params.push(pattern, replace, replaceFlags)

const query = `
UPDATE "${table}"
SET "${column}" = regexp_replace("${column}", $1, $2, $3)
WHERE "${column}" IS NOT NULL
AND regexp_matches("${column}", $1, '${matchFlags}')
`

return { query, params }
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Avoid embedding regex flags; keep them parameterized or omit when empty.

You interpolate matchFlags into SQL and sometimes pass ''. Prefer either (a) omit the 3rd arg when case‑sensitive, or (b) bind 'i' as a param. The diff above opts to omit for case‑sensitive paths.

🤖 Prompt for AI Agents
In backend/src/services/replace-operation.service.ts around lines 59 to 71, the
SQL currently interpolates matchFlags directly and sometimes inserts an empty
string; instead build the query so the regexp_matches call either omits the
flags argument when caseSensitive (i.e. don't interpolate ''), or pass the flag
as a bound parameter—preferably omit it: when caseSensitive, generate WHERE ...
regexp_matches("${column}", $1) and when not caseSensitive generate WHERE ...
regexp_matches("${column}", $1, $3) while adjusting params to only include
pattern, replace, replaceFlags (and adding matchFlag param only for the
case-insensitive branch), ensuring no empty string is injected into the SQL.

Comment on lines +64 to +71
const query = `
UPDATE "${table}"
SET "${column}" = regexp_replace("${column}", $1, $2, $3)
WHERE "${column}" IS NOT NULL
AND regexp_matches("${column}", $1, '${matchFlags}')
`

return { query, params }
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Blocker: Unsafe identifier interpolation (table/column) enables SQL injection via identifiers.

"${table}" and "$ {column}" are string‑interpolated without escaping. Even with value params bound, identifiers remain attacker-controlled (e.g., crafted projectId/column). Quote/validate identifiers server‑side and reuse the sanitized form in queries.

Apply this diff to introduce a safe identifier helper and use it in all queries:

@@
   private buildParameterizedUpdateQuery(
@@
-    if (wholeWord) {
+    const tableIdent = this.quoteIdent(table)
+    const columnIdent = this.quoteIdent(column)
+    if (wholeWord) {
@@
-      const query = `
-        UPDATE "${table}"
-        SET "${column}" = regexp_replace("${column}", $1, $2, $3)
-        WHERE "${column}" IS NOT NULL
-          AND regexp_matches("${column}", $1, '${matchFlags}')
-      `
+      const query =
+        caseSensitive
+          ? `
+        UPDATE ${tableIdent}
+        SET ${columnIdent} = regexp_replace(${columnIdent}, $1, $2, $3)
+        WHERE ${columnIdent} IS NOT NULL
+          AND regexp_matches(${columnIdent}, $1)
+      `
+          : `
+        UPDATE ${tableIdent}
+        SET ${columnIdent} = regexp_replace(${columnIdent}, $1, $2, $3)
+        WHERE ${columnIdent} IS NOT NULL
+          AND regexp_matches(${columnIdent}, $1, 'i')
+      `
@@
-    if (caseSensitive) {
+    if (caseSensitive) {
@@
-      const query = `
-        UPDATE "${table}"
-        SET "${column}" = replace("${column}", $1, $2)
-        WHERE "${column}" IS NOT NULL
-          AND position($1 in "${column}") > 0
-      `
+      const query = `
+        UPDATE ${tableIdent}
+        SET ${columnIdent} = replace(${columnIdent}, $1, $2)
+        WHERE ${columnIdent} IS NOT NULL
+          AND position($1 in ${columnIdent}) > 0
+      `
@@
-    const query = `
-      UPDATE "${table}"
-      SET "${column}" = regexp_replace("${column}", $1, $2, $3)
-      WHERE "${column}" IS NOT NULL
-        AND regexp_matches("${column}", $1, 'i')
-    `
+    const query = `
+      UPDATE ${tableIdent}
+      SET ${columnIdent} = regexp_replace(${columnIdent}, $1, $2, $3)
+      WHERE ${columnIdent} IS NOT NULL
+        AND regexp_matches(${columnIdent}, $1, 'i')
+    `
@@
   private async countAffectedRows(
@@
-    if (wholeWord) {
+    const tableIdent = this.quoteIdent(table)
+    const columnIdent = this.quoteIdent(column)
+    if (wholeWord) {
@@
-      query = `
-        SELECT COUNT(*) as count FROM "${table}"
-        WHERE "${column}" IS NOT NULL
-          AND regexp_matches("${column}", $1, $2)
-      `
+      query = `
+        SELECT COUNT(*) as count FROM ${tableIdent}
+        WHERE ${columnIdent} IS NOT NULL
+          AND regexp_matches(${columnIdent}, $1, $2)
+      `
@@
-        query = `
-          SELECT COUNT(*) as count FROM "${table}"
-          WHERE "${column}" IS NOT NULL
-            AND position($1 in "${column}") > 0
-        `
+        query = `
+          SELECT COUNT(*) as count FROM ${tableIdent}
+          WHERE ${columnIdent} IS NOT NULL
+            AND position($1 in ${columnIdent}) > 0
+        `
@@
-        query = `
-          SELECT COUNT(*) as count FROM "${table}"
-          WHERE "${column}" IS NOT NULL
-            AND regexp_matches("${column}", $1, 'i')
-        `
+        query = `
+          SELECT COUNT(*) as count FROM ${tableIdent}
+          WHERE ${columnIdent} IS NOT NULL
+            AND regexp_matches(${columnIdent}, $1, 'i')
+        `
@@
   private escapeRegex(str: string): string {
     return str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')
   }
+
+  /**
+   * Safely quotes a SQL identifier and enforces a conservative allowlist.
+   */
+  private quoteIdent(ident: string): string {
+    if (!/^[A-Za-z_][A-Za-z0-9_]*$/.test(ident)) {
+      throw new Error(`Invalid identifier: ${ident}`)
+    }
+    return `"${ident.replace(/"/g, '""')}"`
+  }

Also applies to: 78-85, 93-100, 121-126, 129-134, 138-141

Comment on lines +57 to +68
test('should perform basic replace operation', () => {
expect(
service.performReplace({
table,
column: 'city',
find: 'New York',
replace: 'NYC',
caseSensitive: false,
wholeWord: false,
}),
).resolves.toBe(3)
})
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Tests don’t await promises; assertions may be skipped.

Use await (or return) with .resolves/.rejects so the runner waits for the promise and failures are caught.

Apply this diff:

-  test('should perform basic replace operation', () => {
-    expect(
+  test('should perform basic replace operation', async () => {
+    await expect(
       service.performReplace({
         table,
         column: 'city',
         find: 'New York',
         replace: 'NYC',
         caseSensitive: false,
         wholeWord: false,
       }),
-    ).resolves.toBe(3)
+    ).resolves.toBe(3)
   })
@@
-  test('should perform case-sensitive replace operation', () => {
-    expect(
+  test('should perform case-sensitive replace operation', async () => {
+    await expect(
       service.performReplace({
@@
-    ).resolves.toBe(2) // John Doe and John Johnson
+    ).resolves.toBe(2) // John Doe and Bob Johnson
   })
@@
-  test('should perform case-insensitive replace operation', () => {
-    expect(
+  test('should perform case-insensitive replace operation', async () => {
+    await expect(
       service.performReplace({
@@
-    ).resolves.toBe(1) // john@example.com
+    ).resolves.toBe(1) // john@example.com
   })
@@
-  test('should perform whole word replace operation', () => {
-    expect(
+  test('should perform whole word replace operation', async () => {
+    await expect(
       service.performReplace({
@@
-    ).resolves.toBe(1) // Only "John Doe" should match (whole word "John")
+    ).resolves.toBe(1) // Only "John Doe" should match (whole word "John")
   })
@@
-  test('should handle replace with empty string', () => {
-    expect(
+  test('should handle replace with empty string', async () => {
+    await expect(
       service.performReplace({
@@
-    ).resolves.toBe(3)
+    ).resolves.toBe(3)
   })
@@
-  expect(
-    service.performReplace({
+  await expect(
+    service.performReplace({
       table,
       column: 'email',
       find: '@',
       replace: '[AT]',
       caseSensitive: false,
       wholeWord: false,
     }),
-  ).resolves.toBe(6) // All 6 emails contain @
+  ).resolves.toBe(6) // All 6 emails contain @
@@
-  expect(
-    service.performReplace({
+  await expect(
+    service.performReplace({
       table,
       column: 'name',
       find: "John's",
       replace: "Jonathan's",
       caseSensitive: false,
       wholeWord: false,
     }),
-  ).resolves.toBe(1)
+  ).resolves.toBe(1)
@@
-  expect(
-    service.performReplace({
+  await expect(
+    service.performReplace({
       table,
       column: 'nonexistent_column',
       find: 'test',
       replace: 'replacement',
       caseSensitive: false,
       wholeWord: false,
     }),
-  ).rejects.toThrow()
+  ).rejects.toThrow()
@@
-  expect(
-    service.performReplace({
+  await expect(
+    service.performReplace({
       table: 'nonexistent_table',
       column: 'name',
       find: 'John',
       replace: 'Jonathan',
       caseSensitive: false,
       wholeWord: false,
     }),
-  ).rejects.toThrow()
+  ).rejects.toThrow()
@@
-  expect(
-    service.performReplace({
+  await expect(
+    service.performReplace({
       table,
       column: 'city',
       find: 'NonExistentCity',
       replace: 'NewCity',
       caseSensitive: false,
       wholeWord: false,
     }),
-  ).resolves.toBe(0)
+  ).resolves.toBe(0)

Also applies to: 70-81, 83-94, 96-107, 109-120, 129-139, 148-158, 162-173, 175-186, 188-199

🤖 Prompt for AI Agents
In backend/tests/services/replace-operation.service.test.ts around lines 57-68
(and similarly for ranges 70-81, 83-94, 96-107, 109-120, 129-139, 148-158,
162-173, 175-186, 188-199), the tests call expect(...).resolves/.rejects without
awaiting or returning the promise so the test runner may not wait for
assertions; mark the test callbacks async and either await the expectation
(await expect(...).resolves.toBe(...)) or return the expectation (return
expect(...).resolves.toBe(...)) for each affected test case to ensure the
promise is awaited and failures are caught.

@DaxServer DaxServer merged commit ccc1771 into main Sep 28, 2025
6 checks passed
@DaxServer DaxServer deleted the replace-operation-service branch September 28, 2025 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants