-
Notifications
You must be signed in to change notification settings - Fork 1
feat!: Рефакторинг api #14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThis PR introduces async startup and lifecycle controls in HttpBin, expands response helper utilities, renames internal globals in the main controller, refines CLI option spelling, adds logging to parent-process monitoring, updates tests to new APIs, and modifies packagedef with repository URL and a new dependency. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Tests/Client
participant HB as HttpBin
participant Proc as Service Process
participant Srv as HTTP Server
Dev->>HB: ЗапуститьАсинх() / (синхронно через флаг)
activate HB
HB->>Proc: ЗапуститьПроцесс(Синхронно/Ложь)
activate Proc
Note right of HB: Логирование старта, таймаут запуска
alt Синхронный запуск
HB->>Srv: Проверка готовности (Отвечает?)
Srv-->>HB: Готов/Не готов (до таймаута)
HB-->>Dev: Возврат после готовности
else Асинхронный запуск
HB-->>Dev: Немедленный возврат
end
deactivate HB
Dev->>HB: URL(АдресРесурса)
HB-->>Dev: Полный URL
Dev->>HB: Остановить()
HB->>Proc: Terminate
deactivate Proc
HB-->>Dev: Завершено
Dev->>HB: ОжидатьЗавершения()
HB-->>Dev: Блок до завершения процесса
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/core/Классы/HttpBin.os (1)
291-317: Readiness wait throws on any non-200 status (logic bug).The timeout check uses “... OR ValueFilled(CodeStatus)”, so any non-200 response triggers an immediate exception instead of waiting until the timeout.
Apply this fix to wait only until timeout, while still including status/exception details:
- Если ПрошлоСекунд > _ТаймаутЗапуска Или ЗначениеЗаполнено(КодСостояния) Тогда + Если ПрошлоСекунд > _ТаймаутЗапуска Тогда ТекстОшибки = СтрШаблон( "Превышен таймаут запуска сервиса %1:%2 (%3 сек). " + "Сервис не начал отвечать на запросы в течение отведенного времени.", _ИмяХоста, _Порт, _ТаймаутЗапуска); Если ЗначениеЗаполнено(КодСостояния) Тогда ТекстОшибки = СтрШаблон("%1: |%2", ТекстОшибки, КодыСостоянияHTTP.Представление(КодСостояния) ); Иначе ТекстОшибки = СтрШаблон("%1: |%2", ТекстОшибки, ТекстИсключения ); КонецЕсли; ВызватьИсключение ТекстОшибки; КонецЕсли;src/internal/Классы/ПомощникПодготовкиОтветов.os (3)
84-88: Replace offensive 402 response text.Profanity is unacceptable in user‑visible responses.
- Ответ.ТелоТекст = "Fuck you, pay me!"; - Ответ.Заголовки["x-more-info"] = "http://vimeo.com/22053820"; + Ответ.ТелоТекст = "Payment required"; + Ответ.Заголовки["x-more-info"] = "https://datatracker.ietf.org/doc/html/rfc7231#section-6.5.2";
270-276: Regex pattern string likely invalid due to quote escaping.In OneScript/1C, embed double quotes as "" not ". Fix quoting to avoid parse/runtime errors.
- РегулярноеВыражение = Новый РегулярноеВыражение("\s*(W\/)?\""?([^""]*)\""?\s*"); + РегулярноеВыражение = Новый РегулярноеВыражение("\s*(W/)?""?([^""]*)""?\s*");
371-396: Off‑by‑one in weighted random; zero‑weight edge case.If random equals ОбщийВес, index overflows; when ОбщийВес=0, selection is undefined.
ГСЧ = Новый ГенераторСлучайныхЧисел(); - СлучайноеЧисло = ГСЧ.СлучайноеЧисло(0, ОбщийВес); + Если ОбщийВес = 0 Тогда + Возврат Неопределено; + КонецЕсли; + СлучайноеЧисло = ГСЧ.СлучайноеЧисло(0, ОбщийВес - 1); НижняяГраница = 0; ВерхняяГраница = Список.Количество();
🧹 Nitpick comments (7)
src/core/Классы/HttpBin.os (5)
85-95: Readiness probe fails when host is 0.0.0.0.Probing http://0.0.0.0:port won’t work. Map 0.0.0.0 to 127.0.0.1 for checks.
Попытка - Ответ = КоннекторHTTP.Head(URL(), Новый Структура("Таймаут", _ТаймаутПроверки)); + АдресПроверки = ?(_ИмяХоста = "0.0.0.0", "127.0.0.1", _ИмяХоста); + URLПроверки = СтрШаблон("http://%1:%2/", АдресПроверки, _Порт); + Ответ = КоннекторHTTP.Head(URLПроверки, Новый Структура("Таймаут", _ТаймаутПроверки));
272-281: Also use 127.0.0.1 for readiness checks when bound to 0.0.0.0.Ensures logs and HEAD use a reachable address.
- - _Лог.Отладка("Проверка готовности сервиса %1. Попытка: %2", URL(), НомерПопытки); + АдресПроверки = ?(_ИмяХоста = "0.0.0.0", "127.0.0.1", _ИмяХоста); + URLПроверки = СтрШаблон("http://%1:%2/", АдресПроверки, _Порт); + _Лог.Отладка("Проверка готовности сервиса %1. Попытка: %2", URLПроверки, НомерПопытки); Попытка - Ответ = КоннекторHTTP.Head(URL(), Новый Структура("Таймаут", _ТаймаутПроверки)); + Ответ = КоннекторHTTP.Head(URLПроверки, Новый Структура("Таймаут", _ТаймаутПроверки));
47-51: Consider waiting for process termination after stopping.Avoid zombie processes by waiting briefly after Завершить().
Если Активен() Тогда _Процесс.Завершить(); + _Процесс.ОжидатьЗавершения(); КонецЕсли;
236-239: Unify quoting for cross‑platform process start.Single quotes on *nix may be unsafe depending on CreateProcess/exec parsing. Prefer double quotes consistently for paths with spaces.
СтрокаКоманды.Добавить(ОбернутьВДвойныеКавычки(ИсполняемыйФайл)); - СтрокаКоманды.Добавить(ОбернутьВКавычки(ТочкаВходаКонсольногоПриложения())); + СтрокаКоманды.Добавить(ОбернутьВДвойныеКавычки(ТочкаВходаКонсольногоПриложения()));
331-346: Name shadowing reduces clarity.Local variable КаталогПрограммы shadows function КаталогПрограммы(). Rename local var to avoid confusion.
src/internal/Классы/ПомощникПодготовкиОтветов.os (2)
481-489: Use first X-Forwarded-For value and trim spaces.Header can contain a list; return the client’s IP (leftmost).
Адрес = ЗначениеЗаголовка(Запрос.Заголовки, "X-Forwarded-For"); - Если Не ЗначениеЗаполнено(Адрес) Тогда + Если ЗначениеЗаполнено(Адрес) Тогда + Список = СтрРазделить(Адрес, ","); + Адрес = СокрЛП(СокрП(Список[0])); + Иначе Адрес = Запрос.АдресУдаленногоУзла; КонецЕсли;
269-289: Typo in function name “РаспаристьМногозначныйЗаголовок”.Rename to “РаспарситьМногозначныйЗаголовок” for correctness and readability; update call sites.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
packagedef(2 hunks)src/app/ОсновнойКонтрол.os(28 hunks)src/cmd/Классы/КомандаЗапустить.os(1 hunks)src/cmd/Классы/КонтроллерРодительскогоПроцесса.os(2 hunks)src/core/Классы/HttpBin.os(3 hunks)src/internal/Классы/ПомощникПодготовкиОтветов.os(16 hunks)tests/HttpBin_API_test.os(2 hunks)tests/HttpBin_test.os(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: test (stable) / build (ubuntu-latest, stable)
- GitHub Check: test (stable) / build (macos-latest, stable)
- GitHub Check: test (stable) / build (macos-latest, stable)
- GitHub Check: test (stable) / build (ubuntu-latest, stable)
- GitHub Check: sonar / test
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores