9 спринт. 1-коммит.#8
Open
Georgiy2803 wants to merge 6 commits intomainfrom
Open
Conversation
Reviewer's GuideThis PR adds a full HTTP API layer to the task manager (with server, handlers and JSON adapters) and coverage tests, refactors CSV conversion logic, and extends the Main class with manual in-memory/file-backed manager scenarios. Sequence diagram for HTTP request handling in TaskHandlersequenceDiagram
actor Client
participant HttpTaskServer
participant TaskHandler
participant TaskManager
participant Gson
Client->>HttpTaskServer: HTTP request (e.g. POST /tasks)
HttpTaskServer->>TaskHandler: handle(exchange)
TaskHandler->>Gson: Parse request body (fromJson)
TaskHandler->>TaskManager: createTask(task)
TaskManager-->>TaskHandler: Task created
TaskHandler->>Gson: Serialize response (toJson)
TaskHandler->>Client: HTTP response
Class diagram for new HTTP API handlers and adaptersclassDiagram
class BaseHttpHandler {
- TaskManager taskManager
- Gson gson
+ sendText()
+ sendOk()
+ sendBadRequest()
+ sendNotFound()
+ sendHasInteractions()
+ sendCreated()
+ sendInternalServerError()
+ sendMethodNotAllowed()
+ searchIdTask()
}
class TaskHandler {
+ handle()
+ handleGetTasks()
+ handleGetTaskById()
+ handleCreateTask()
+ handleDeleteTask()
}
class SubtaskHandler {
+ handle()
+ handleGetSubtasks()
+ handleGetSubtaskId()
+ handleCreateSubtask()
+ handleDeleteSubtask()
}
class EpicHandler {
+ handle()
+ handleGetEpics()
+ handleGetEpicId()
+ handleGetEpicSubtasks()
+ handleCreateEpic()
+ handleDeleteEpic()
}
class HistoryHandler {
+ handle()
}
class PrioritizedHandler {
+ handle()
}
class DurationAdapter {
+ write()
+ read()
}
class LocalDateTimeAdapter {
+ write()
+ read()
}
TaskHandler --|> BaseHttpHandler
SubtaskHandler --|> BaseHttpHandler
EpicHandler --|> BaseHttpHandler
HistoryHandler --|> BaseHttpHandler
PrioritizedHandler --|> BaseHttpHandler
Class diagram for updated CsvConverter time processingclassDiagram
class CsvConverter {
+ processingTimeForTask(Task, String, String)
+ processingTimeForEpic(Epic, String)
+ stringToDate(String)
+ stringToDuration(String)
- isNoData(String)
}
CsvConverter ..> Task
CsvConverter ..> Epic
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @Georgiy2803 - I've reviewed your changes and found some issues that need to be addressed.
Blocking issues:
- Missing break statements in switch may cause fall-through. (link)
General comments:
- Main.java contains huge commented-out blocks—please remove or relocate example code into proper tests or demos to keep the entry point clean.
- EpicHandler.handle() is missing break statements in its switch, resulting in unintended fall-through; add breaks to ensure only the correct endpoint logic runs.
- HttpTaskServer.main() starts and then immediately stops the server—remove the premature stopServer() call so the server stays running as expected.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Main.java contains huge commented-out blocks—please remove or relocate example code into proper tests or demos to keep the entry point clean.
- EpicHandler.handle() is missing break statements in its switch, resulting in unintended fall-through; add breaks to ensure only the correct endpoint logic runs.
- HttpTaskServer.main() starts and then immediately stops the server—remove the premature stopServer() call so the server stays running as expected.
## Individual Comments
### Comment 1
<location> `src/main/api/handler/EpicHandler.java:27` </location>
<code_context>
+ public void handle(HttpExchange exchange) throws IOException {
+ Endpoint endpoint = getEndpoint(exchange.getRequestURI(), exchange.getRequestMethod());
+
+ switch (endpoint) {
+ case GET_EPIC:
+ handleGetEpicId(exchange);
+ case GET_EPICS:
+ handleGetEpics(exchange);
+ case GET_EPIC_SUBTASKS:
+ handleGetEpicSubtasks(exchange);
+ case POST_EPIC:
+ handleCreateEpic(exchange);
+ case DELETE_EPIC:
+ handleDeleteEpic(exchange);
+ case UNKNOWN:
+ sendMethodNotAllowed(exchange);
+ }
</code_context>
<issue_to_address>
Missing break statements in switch may cause fall-through.
The switch cases in handle() lack break statements, causing all subsequent handlers to execute after a match. This can lead to multiple responses for one request.
</issue_to_address>
### Comment 2
<location> `src/main/api/adapter/DurationAdapter.java:26` </location>
<code_context>
+ }
+
+ @Override
+ public Duration read(final JsonReader jsonReader) throws IOException {
+ long minutes = Long.parseLong(jsonReader.nextString());
+ return Duration.ofMinutes(minutes);
+ }
+}
</code_context>
<issue_to_address>
No handling for empty or invalid input in read().
Consider adding error handling for cases where nextString() returns an empty or invalid value, such as returning null or throwing a custom exception, to prevent runtime errors.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
@Override
public Duration read(final JsonReader jsonReader) throws IOException {
long minutes = Long.parseLong(jsonReader.nextString());
return Duration.ofMinutes(minutes);
}
=======
@Override
public Duration read(final JsonReader jsonReader) throws IOException {
String value = jsonReader.nextString();
if (value == null || value.trim().isEmpty()) {
return null;
}
try {
long minutes = Long.parseLong(value);
return Duration.ofMinutes(minutes);
} catch (NumberFormatException e) {
// Optionally, you could throw a custom exception here instead of returning null
return null;
}
}
>>>>>>> REPLACE
</suggested_fix>
### Comment 3
<location> `src/main/api/adapter/LocalDateTimeAdapter.java:33` </location>
<code_context>
+ }
+
+ @Override
+ public LocalDateTime read(final JsonReader jsonReader) throws IOException {
+ String startTimeStr = jsonReader.nextString();
+ if (!startTimeStr.isEmpty()) {
+ return LocalDateTime.parse(startTimeStr, dtf);
+ }
+ return null;
+ }
+
</code_context>
<issue_to_address>
No error handling for invalid date format in read().
Add error handling to manage exceptions from LocalDateTime.parse() when input does not match the expected format.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
@Override
public LocalDateTime read(final JsonReader jsonReader) throws IOException {
String startTimeStr = jsonReader.nextString();
if (!startTimeStr.isEmpty()) {
return LocalDateTime.parse(startTimeStr, dtf);
}
return null;
}
=======
@Override
public LocalDateTime read(final JsonReader jsonReader) throws IOException {
String startTimeStr = jsonReader.nextString();
if (!startTimeStr.isEmpty()) {
try {
return LocalDateTime.parse(startTimeStr, dtf);
} catch (Exception e) {
throw new IOException("Invalid date format for LocalDateTime: " + startTimeStr, e);
}
}
return null;
}
>>>>>>> REPLACE
</suggested_fix>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+27
to
+40
| switch (endpoint) { | ||
| case GET_EPIC: | ||
| handleGetEpicId(exchange); | ||
| case GET_EPICS: | ||
| handleGetEpics(exchange); | ||
| case GET_EPIC_SUBTASKS: | ||
| handleGetEpicSubtasks(exchange); | ||
| case POST_EPIC: | ||
| handleCreateEpic(exchange); | ||
| case DELETE_EPIC: |
There was a problem hiding this comment.
issue (bug_risk): Missing break statements in switch may cause fall-through.
The switch cases in handle() lack break statements, causing all subsequent handlers to execute after a match. This can lead to multiple responses for one request.
Comment on lines
+25
to
+29
| @Override | ||
| public Duration read(final JsonReader jsonReader) throws IOException { | ||
| long minutes = Long.parseLong(jsonReader.nextString()); | ||
| return Duration.ofMinutes(minutes); | ||
| } |
There was a problem hiding this comment.
suggestion: No handling for empty or invalid input in read().
Consider adding error handling for cases where nextString() returns an empty or invalid value, such as returning null or throwing a custom exception, to prevent runtime errors.
Suggested change
| @Override | |
| public Duration read(final JsonReader jsonReader) throws IOException { | |
| long minutes = Long.parseLong(jsonReader.nextString()); | |
| return Duration.ofMinutes(minutes); | |
| } | |
| @Override | |
| public Duration read(final JsonReader jsonReader) throws IOException { | |
| String value = jsonReader.nextString(); | |
| if (value == null || value.trim().isEmpty()) { | |
| return null; | |
| } | |
| try { | |
| long minutes = Long.parseLong(value); | |
| return Duration.ofMinutes(minutes); | |
| } catch (NumberFormatException e) { | |
| // Optionally, you could throw a custom exception here instead of returning null | |
| return null; | |
| } | |
| } |
Comment on lines
+32
to
+34
| @Override | ||
| public LocalDateTime read(final JsonReader jsonReader) throws IOException { | ||
| String startTimeStr = jsonReader.nextString(); | ||
| if (!startTimeStr.isEmpty()) { | ||
| return LocalDateTime.parse(startTimeStr, dtf); | ||
| } | ||
| return null; | ||
| } |
There was a problem hiding this comment.
suggestion (bug_risk): No error handling for invalid date format in read().
Add error handling to manage exceptions from LocalDateTime.parse() when input does not match the expected format.
Suggested change
| @Override | |
| public LocalDateTime read(final JsonReader jsonReader) throws IOException { | |
| String startTimeStr = jsonReader.nextString(); | |
| if (!startTimeStr.isEmpty()) { | |
| return LocalDateTime.parse(startTimeStr, dtf); | |
| } | |
| return null; | |
| } | |
| @Override | |
| public LocalDateTime read(final JsonReader jsonReader) throws IOException { | |
| String startTimeStr = jsonReader.nextString(); | |
| if (!startTimeStr.isEmpty()) { | |
| try { | |
| return LocalDateTime.parse(startTimeStr, dtf); | |
| } catch (Exception e) { | |
| throw new IOException("Invalid date format for LocalDateTime: " + startTimeStr, e); | |
| } | |
| } | |
| return null; | |
| } |
Georgiy2803
force-pushed
the
sprint_9-solution-http-api
branch
from
cc1b7f6 to
d3ae0bf
Compare
Georgiy2803
force-pushed
the
sprint_9-solution-http-api
branch
from
d3ae0bf to
cc5ff91
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.
Summary by Sourcery
Set up a full HTTP-based interface for the task management system with Gson-backed handlers and adapters, streamline CSV time parsing, add comprehensive integration tests for all endpoints, and include demo code in Main.java
New Features:
Enhancements:
Tests:
Chores: