solved the task#238
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because there are still a couple of requirement-level issues that will likely cause the tests to fail.
Critical issues to fix:
- In
AsyncRequestProcessor.processRequest(filesrc/main/java/mate/academy/AsyncRequestProcessor.java), you always callCompletableFuture.supplyAsync(...)and only access the cache inside the async task. This violates the requirement that cached results must be returned immediately. You should first do something like:UserData cached = cache.get(userId);- if not null,
return CompletableFuture.completedFuture(cached); - only if it’s absent, schedule
supplyAsyncto compute, put into the cache, and return.
- The
UserDatadetails string does not match the expected format. The requirement is that the details look like"Details for user1", so when constructingUserDatayou should use"Details for " + userId, and ensureUserData.toString()prints exactlyUserData[userId=..., details=Details for ...]. - The task requires a
UserDataclass withuserId,details, and the specifictoString()format; make sure this class exists insrc/main/java/mate/academy/(or the appropriate package) and that its fields andtoString()are implemented to match the expected console output.
Minor suggestions (optional):
- Consider restoring the interrupt flag if
Thread.sleepis interrupted (callThread.currentThread().interrupt()before rethrowing), and optionally guardexecutor.shutdown()inMainwith atry/finally.
You’re very close: the use of CompletableFuture, the executor, and the concurrent cache are all on the right track—once you fix the synchronous cache hit behavior and output formatting, your solution should fully meet the requirements.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| return CompletableFuture.supplyAsync( | ||
| () -> cache.computeIfAbsent( | ||
| userId, | ||
| id -> { | ||
| try { | ||
| Thread.sleep(200); | ||
| } catch (InterruptedException e) { | ||
| throw new RuntimeException(e); | ||
| } | ||
| return new UserData(id, "info about user with id: " + id); | ||
| } | ||
| ), executor | ||
| ); |
There was a problem hiding this comment.
The current implementation always calls CompletableFuture.supplyAsync and runs cache.computeIfAbsent inside the async task. This means cached entries still cause a task to be submitted instead of returning immediately. The requirements state: check the cache first and return the cached result immediately. Consider: UserData cached = cache.get(userId); if (cached != null) return CompletableFuture.completedFuture(cached); then schedule the async work only when absent.
| public CompletableFuture<UserData> processRequest(String userId) { | ||
| return null; | ||
| return CompletableFuture.supplyAsync( | ||
| () -> cache.computeIfAbsent( |
There was a problem hiding this comment.
Using computeIfAbsent inside the supplier delays cache checking until the worker thread runs. If you prefer to keep a single atomic compute, consider using computeIfAbsent from the calling thread only with a mapping that returns a completed value, or more simply perform a synchronous get check and then supplyAsync to compute-and-put. Be careful not to block caller threads with long sleeps inside computeIfAbsent if you change placement.
| } catch (InterruptedException e) { | ||
| throw new RuntimeException(e); | ||
| } | ||
| return new UserData(id, "info about user with id: " + id); |
There was a problem hiding this comment.
The string used to create UserData is "info about user with id: " + id, but the task expects Details for user1 style output. Change to "Details for " + id (and ensure UserData.toString() prints exactly UserData[userId=..., details=Details for ...]).
|
|
||
| public class AsyncRequestProcessor { | ||
| private final Executor executor; | ||
| private Map<String, UserData> cache = new ConcurrentHashMap<>(); |
There was a problem hiding this comment.
Cache declaration is correct (ConcurrentHashMap). I couldn't find the UserData class implementation in the provided files — the task requires a UserData class with userId and details and a toString() that matches the expected console output. Add or verify that class exists and formats its output as required.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because there is a critical missing piece that prevents the project from compiling and the task from being fully completed. Your AsyncRequestProcessor and Main logic look good: you use CompletableFuture, a ConcurrentHashMap cache, simulate delay with Thread.sleep(200), build details as "Details for " + userId, and correctly restore the interrupt flag on InterruptedException. However, the required UserData class is not present under package mate.academy, so any reference like new UserData(userId, details) will fail to compile and the expected console output format cannot be produced.
Critical issues to fix:
- Add
src/main/java/mate/academy/UserData.javawith a constructorUserData(String userId, String details)and implementtoString()to return exactlyUserData[userId=<id>, details=Details for <id>](for example, foruser1it must beUserData[userId=user1, details=Details for user1]). This is required so tests that compare output strings pass.
As a possible improvement (not required to pass): consider how to avoid duplicate work when two concurrent calls request the same userId before the first finishes, e.g., by keeping a map of in-flight CompletableFuture<UserData> in addition to the required Map<String, UserData> cache = new ConcurrentHashMap<>(). Overall your async logic is on the right track; once you add the UserData class with the exact signature and toString format, your solution should be in good shape.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| Thread.currentThread().interrupt(); | ||
| throw new RuntimeException(e); | ||
| } | ||
| UserData newUserData = new UserData(userId, "Details for " + userId); |
There was a problem hiding this comment.
You create a new UserData instance here; ensure a UserData class exists in package mate.academy with a constructor UserData(String userId, String details) and a toString() that returns exactly UserData[userId=<id>, details=Details for <id>]. Without that class (or incorrect toString) the code/tests will fail to compile or the output will not match the expected format from the task description.
| throw new RuntimeException(e); | ||
| } | ||
| UserData newUserData = new UserData(userId, "Details for " + userId); | ||
| cache.put(userId, newUserData); |
There was a problem hiding this comment.
Because you populate the cache only after the async task completes, two concurrent requests for the same userId that arrive while the first is still processing will both start work (duplicate processing). If tests require immediate reuse for repeated requests issued while the first is in-flight, consider also caching in-flight CompletableFuture<UserData> instances (or use an additional in-flight map) to de-duplicate; note that the assignment explicitly requires Map<String, UserData> cache = new ConcurrentHashMap<>(), so add any extra structure without removing the required cache declaration.
| } catch (InterruptedException e) { | ||
| Thread.currentThread().interrupt(); |
There was a problem hiding this comment.
You catch InterruptedException and restore the interrupt flag before wrapping and rethrowing — good practice. No change required, but keep this behavior (it helps tests that check interruption handling).
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Excellent work on this iteration! You've successfully addressed all the previous feedback by adding the UserData class, which makes the entire solution complete and functional. Your implementation of asynchronous processing with CompletableFuture and the caching mechanism using ConcurrentHashMap is spot-on. The code is clean, correct, and fully meets all the requirements. I'm happy to approve your submission, great job!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
2 participants
No description provided.