Skip to content

refactor: improve IPC handler structure and validation#96

Merged
KazeKaze93 merged 1 commit intomasterfrom
refactor/ipc-validation-dedup
Apr 28, 2026
Merged

refactor: improve IPC handler structure and validation#96
KazeKaze93 merged 1 commit intomasterfrom
refactor/ipc-validation-dedup

Conversation

@KazeKaze93
Copy link
Copy Markdown
Owner

  • Refactored SearchController to utilize dedicated methods for resolving copyright tags and handling tag type resolution, enhancing code clarity and maintainability.
  • Simplified the MaintenanceController by directly accepting the AutoBackupInterval parameter, removing unnecessary argument parsing.
  • Introduced createValidatedHandler in updates.ts to streamline validation and error handling for the GET_TOTAL_UNREAD_COUNT IPC call, ensuring consistent input validation across handlers.
  • Cleaned up unused imports and ensured adherence to strict coding standards throughout the changes.

- Refactored `SearchController` to utilize dedicated methods for resolving copyright tags and handling tag type resolution, enhancing code clarity and maintainability.
- Simplified the `MaintenanceController` by directly accepting the `AutoBackupInterval` parameter, removing unnecessary argument parsing.
- Introduced `createValidatedHandler` in `updates.ts` to streamline validation and error handling for the `GET_TOTAL_UNREAD_COUNT` IPC call, ensuring consistent input validation across handlers.
- Cleaned up unused imports and ensured adherence to strict coding standards throughout the changes.
@KazeKaze93 KazeKaze93 merged commit ad24a9f into master Apr 28, 2026
1 of 2 checks passed
@github-actions
Copy link
Copy Markdown

🛡️ Архитектор (AI Review)

Это Code Review. Пристегнись, будет больно. Ты называешь это «рефакторингом», я называю это «попыткой протащить блокирующие вызовы и небезопасный ввод в Main-процесс».

🚨 Критично

1. Блокировка Main-процесса (Performance)
В файле src/main/ipc/handlers/updates.ts:

const row = getDb()
  .select({ value: count() })
  .from(posts)
  .where(whereClause)
  .get(); // <--- СМЕРТЬ

Ты используешь .get() в Drizzle с драйвером better-sqlite3. Это синхронный вызов. Если база разрастется или диск будет занят, весь UI Electron-приложения зафризится, потому что Main-процесс будет ждать ответа от БД.
Исправление: Используй асинхронные методы (если драйвер позволяет) или выноси тяжелые запросы в Worker Thread. В текущем стеке — как минимум не делай вид, что Promise.resolve(row) что-то спасает, когда поток уже заблокирован на строке выше.

2. Слепое доверие к типам в MaintenanceController
В файле src/main/ipc/controllers/MaintenanceController.ts:

-    const interval = AutoBackupIntervalSchema.parse(args[0]);
+    interval: AutoBackupInterval

Ты удалил валидацию Zod внутри метода. Если твой BaseController.handle просто прокидывает args[0] и кастует его к типу через as, то ты открыл дыру. Renderer может прислать любой мусор в interval, и если scheduleAutoBackup ожидает число, а получит undefined или string, ты поймаешь крэш в Main-процессе.
Правило: Никогда не доверяй типам TypeScript на границе IPC. Только рантайм-валидация (Zod).


⚠️ Надо исправить

1. Избыточный Promise.resolve
В файле src/main/ipc/handlers/updates.ts:

return Promise.resolve(row?.value ?? 0);

Зачем здесь Promise.resolve? Если функция обернута в maintenanceQueue.execute, которая, скорее всего, возвращает Promise, или сам хэндлер асинхронный — это лишний мусор в коде.

2. Обработка ошибок в createValidatedHandler

throw new Error(`Invalid IPC payload for ${channel}`);

Выбрасывание ошибки в IPC-хэндлере без специальной обработки на стороне Renderer приведет к generic-ошибке "Invoke Error". Лучше возвращать объект с ошибкой или использовать кастомный класс исключений, который корректно сериализуется.


💡 Совет

1. Оптимизация Drizzle запросов
В updates.ts ты делаешь innerJoin с artists. Убедись, что на колонках posts.artistId и artists.id стоят индексы. Для SQLite это критично при выполнении count().

2. DRY в SearchController
Методы resolveCopyrightTags и resolveTagsByTypeHandler — это просто прокси.

private resolveCopyrightTags(event: IpcMainInvokeEvent, tags: string[]) {
  return this.resolveTagsByType(event, tags, TAG_TYPES.COPYRIGHT);
}

Это чище, чем было, но убедись, что this.handle в BaseController действительно корректно биндит контекст, иначе this.resolveTagsByType отвалится с undefined. Использование .bind(this) в конструкторе — ок, но следи за производительностью, если хэндлеров сотни.

3. Логирование в createValidatedHandler

log.error(`[IPC] Invalid payload for ${channel}:`, validationResult.error.flatten());

Хорошо, что добавил flatten(), но не забудь, что в продакшене логи не должны содержать чувствительные данные пользователей, которые могут оказаться в rawPayload.


Итог:
Идея с createValidatedHandlerхорошо.
Синхронный .get() в Main-процессе — плохо.
Удаление рантайм-валидации в MaintenanceControllerопасно.

Исправляй блокирующие вызовы БД, пока я не отправил этот PR в /dev/null. 🟢 (Почти, но нет).

Analyzed by: gemini-3-flash-preview

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.

1 participant