-
Notifications
You must be signed in to change notification settings - Fork 0
Sprint 9 solution http api #9
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
base: sprint_8-solution-time-and-duration
Are you sure you want to change the base?
Sprint 9 solution http api #9
Conversation
…history,/prioritized (sprint 9)
…ектно чистить по id (sprint 9)
…ackedTaskManager(...)
…безопасная работа с сабами/пересчёт; Task — косметика
…держка -DBASE_URL
…роверка 201/200/404/406 и уникальности в prioritized
LexLippi
left a comment
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.
Привет!
Неплохая работа!
Есть некоторое количество замечаний, которые необходимо исправить!
Желаю удачи!
| /* ===== helpers ===== */ | ||
|
|
||
| private HttpResponse<String> get(String path) throws IOException, InterruptedException { | ||
| return client() | ||
| .send( | ||
| HttpRequest.newBuilder(URI.create(baseUrl + path)).GET().build(), | ||
| HttpResponse.BodyHandlers.ofString()); | ||
| } | ||
|
|
||
| private HttpResponse<String> post(String path, String body) | ||
| throws IOException, InterruptedException { | ||
| return client() | ||
| .send( | ||
| HttpRequest.newBuilder(URI.create(baseUrl + path)) | ||
| .header("Content-Type", "application/json") | ||
| .POST(HttpRequest.BodyPublishers.ofString(body)) | ||
| .build(), | ||
| HttpResponse.BodyHandlers.ofString()); | ||
| } | ||
|
|
||
| private HttpResponse<String> delete(String path) throws IOException, InterruptedException { | ||
| return client() | ||
| .send( | ||
| HttpRequest.newBuilder(URI.create(baseUrl + path)).DELETE().build(), | ||
| HttpResponse.BodyHandlers.ofString()); | ||
| } |
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.
Эти хелперы очень похожи лучше выгести их в отдельный утилитарный класс, чтобы не копипастить
| private static int firstIdFromArray(String json) { | ||
| JsonArray arr = JsonParser.parseString(json).getAsJsonArray(); | ||
| JsonObject o = arr.get(0).getAsJsonObject(); | ||
| return o.get("id").getAsInt(); | ||
| } | ||
|
|
||
| private static HttpClient client() { | ||
| return HttpClient.newHttpClient(); | ||
| } |
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.
Аналогично эти методы тоже хочется куда-то вынести, чтобы не копипастить
| dur, | ||
| st, | ||
| "" // epic | ||
| protected int id; |
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.
Лучше не совмещать такие коммиты в одном PR.
Часто в рабочих проектах есть отдельные задачи на форматирование кода и их не объединяют с другими, чтобы не захламлять осмысленные PR украшательствами всех файлов проекта, потому что в случае необходимости откатить изменения нам придется откатывать много чего лишнего (что может сказаться на скорости)
src/http/TasksHandler.java
Outdated
| return id == null || id == 0; | ||
| } | ||
|
|
||
| private Integer parseId(String s) { |
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.
- Этот метод может быть статическим и лучше вынести его в отдельный класс с утилитами
- Стоит назвать его поконкретнее, например
parseIdWithoutException
src/http/TasksHandler.java
Outdated
| sendOk(exchange, "\"deleted\""); | ||
| } | ||
|
|
||
| private boolean isNewId(Integer id) { |
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.
Аналогичное замечание, как и к методу ниже
src/http/TasksHandler.java
Outdated
| manager.addNewTask(incoming); | ||
| sendCreated(exchange); | ||
| } else { | ||
| // sprint9: обновляем ТОЛЬКО существующую задачу — иначе 404 |
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.
Эту логику лучше не накладывать на хэндлер, а имплементировать ее внутри менеджера. Например, выбрасывать исключение, если не нашли обновляемую задачу, а здесь это исключение перебрасывать в sendNotFound
src/http/TasksHandler.java
Outdated
| if (task == null) { | ||
| sendNotFound(exchange, "task " + id + " not found"); | ||
| } else { | ||
| sendOk(exchange, gson.toJson(task)); | ||
| } |
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.
Аналогично, лучше такую логику реализовывать на уровне менеджера
src/http/SubtasksHandler.java
Outdated
| private boolean isNewId(Integer id) { | ||
| return id == null || id == 0; | ||
| } | ||
|
|
||
| private Integer parseId(String s) { | ||
| try { | ||
| return Integer.parseInt(s); | ||
| } catch (NumberFormatException e) { | ||
| return null; | ||
| } | ||
| } |
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.
про это уже писал
LexLippi
left a comment
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.
Привет!
Хорошая работа!
Желаю удачи в следующих спринтах!
Fix
Почистил комментарии по проекту: упростил формулировки, убрал «украшения», оставил только смысл.
Вынес общие хелперы HTTP:
HttpUtil.isNewId,HttpUtil.parseIdOrNull; удалил дубли приватных методов в хендлерах.Хендлеры (
TasksHandler,SubtasksHandler,EpicsHandler):get/post/delete/firstId.../client,import static http.TestHttpUtils.*в тестах,NotFoundException→ возвращаем 404, пересечения времени → 406.Менеджер (
InMemoryTaskManager):get*/update*/remove*кидаютNotFoundException,addNewSubtaskкидаетNotFoundException("epic X not found"),FileBackedTaskManager:durationMinutes,startTime(ISO),put*PreserveId+setNextIdAfterRestore(maxId+1).Тесты:
src/test/java, удалил старые пути,BaseHttpTestподдерживает внешний сервер через-DBASE_URL.TestHttpUtils (тестовые хелперы)
Единая обёртка над
HttpClientдля тестов: меньше шума и дублирования.Методы (static, импортируются через
import static http.TestHttpUtils.*):get(baseUrl, path)post(baseUrl, path, json)— ставитContent-Type: application/jsondelete(baseUrl, path)firstIdFromArray(jsonArrayString)— быстрый парс первогоidиз JSON-массива.Не завязан на сервере/менеджере, работает только по HTTP;
baseUrlберётся изBaseHttpTest.Результат — компактные и читаемые тесты без локальных дубликатов
get/post/delete.Поведение API (кратко)
{id}не существует (дляGET/POST(обновление)/DELETEна/tasks|/epics|/subtasks/{id}).POST/обновленияtasks|subtasks.startTime— ISOyyyy-MM-dd'T'HH:mm[:ss],duration— минуты (целое число).Проверка “пересечения” (Windows
cmd.exe)полезно для контроля:
Зачем
Sprint 9: HTTP API for Kanban Tracker
Цель: открыть функциональность
TaskManagerпо HTTP. Пользователь может работать с задачами, подзадачами, эпиками, историей и приоритезированным списком через REST.Что сделано
Добавлен HTTP-сервер
HttpTaskServer(порт по умолчанию 8080, поддержка запуска на произвольном порту в тестах).Хендлеры:
/tasks—GET,POST,DELETE /{id}/subtasks—GET,POST,DELETE /{id}/epics—GET,POST,DELETE /{id}, а такжеGET /epics/{id}/subtasks/history—GET/prioritized—GETБазовый класс хендлеров
BaseHttpHandlerс единообразной отправкой ответов:200/201/404/406/500.JSON через Gson (
library/gson-2.11.0.jarв репозитории). Адаптеры:LocalDateTime↔ ISO-строка (yyyy-MM-dd'T'HH:mm:ss)Duration↔ миллисекундыУлучшения менеджера:
/prioritizedприadd/update/remove;Модели:
Epic— безопасная работа со списком сабтасков, пересчёт статуса/времени/длительности;Subtask— удобные конструкторы сDurationиLocalDateTime.Тестовая инфраструктура:
BaseHttpTest— автоподнятие локального сервера на свободном порту (0) + поддержка-DBASE_URL=http://localhost:8080для прогона против внешнего инстанса.406на пересечения и уникальность в/prioritized.Карта API
Задачи
GET /tasks— списокGET /tasks/{id}— по idPOST /tasks— создать/обновить (еслиidотсутствует — create; иначе update)DELETE /tasks/{id}— удалитьПодзадачи
GET /subtasksGET /subtasks/{id}POST /subtasksDELETE /subtasks/{id}Эпики
GET /epicsGET /epics/{id}POST /epicsDELETE /epics/{id}GET /epics/{id}/subtasks— все сабтаски эпикаИстория/приоритет
GET /historyGET /prioritizedКоды ответа
200 OK— успешный запрос с телом201 Created— успешныйPOSTбез сущности в ответе404 Not Found— объект или путь не найдены406 Not Acceptable— пересечение по времени500 Internal Server Error— иные ошибки обработкиПримеры запросов (curl)
Как запускать
Из IDE:
http.HttpTaskServer.main().Файл
tasks.csvсоздаётся при первом изменении данных и пишется в Working directory.Тесты:
по умолчанию поднимается временный сервер на порту
0(чистое окружение);для прогона против запущенного инстанса добавь VM-опцию:
Замечания по совместимости
FileBackedTaskManagerпродолжает работать.Проверено
404/406, сортировка/prioritized, заполнение/history.