Test/unit tests vet controller

[TatjanaTrajkovic]

Summary by CodeRabbit

• Tests

  • Added comprehensive test suite for vet management controller covering creation, retrieval, authorization enforcement, input validation, and error handling scenarios.

• Chores

  • Added Jackson databind dependency for JSON serialization support.

[coderabbitai]

📝 Walkthrough

Walkthrough

The changes add a new Jackson databind dependency to the Maven configuration and introduce a comprehensive Spring MVC test class for the VetController, covering creation, retrieval, authorization, validation, and error-handling scenarios across multiple HTTP endpoints.

Changes

Cohort / File(s)	Summary
Build Configuration pom.xml	Added jackson-databind Maven dependency for JSON serialization/deserialization support.
Controller Tests src/test/java/org/example/vet1177/controller/VetControllerTest.java	New Spring MVC test class with 6 test methods covering POST (create vet with success/authorization/validation checks), GET (list all vets), and GET (retrieve by ID with success/not-found handling) endpoints.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

• test: add unit tests for AdminPolicy #166: Adds direct unit tests for AdminPolicy.requireAdmin, which is mocked/stubbed in the new controller tests.
• feat: implement VetController and finalize Vet API endpoints. Closes #53 #72: Implements the VetController endpoints and method signatures that are now covered by the new test class.

Suggested reviewers

• annikaholmqvist94
• lindaeskilsson

Poem

🐰 A test for every endpoint, oh what joy!
Jackson binds the JSON, no ploy.
Controllers tested, vets retrieved with care,
Admin checks and errors handled fair!
The code hops forward, quality in the air! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: adding unit tests for the VetController, which is the primary purpose of this PR.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch test/unit_tests_vet_controller

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (3)

src/test/java/org/example/vet1177/controller/VetControllerTest.java (3)

99-111: Strengthen negative-path assertions with error payload checks.

These tests currently validate only status codes. Adding JSON assertions (e.g., $.status, $.message) would better protect GlobalExceptionHandler contract regressions.

Also applies to: 164-172

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/test/java/org/example/vet1177/controller/VetControllerTest.java` around
lines 99 - 111, The tests (e.g., shouldReturn400_whenInvalidRequest and the
other negative-path test around lines 164-172) only assert HTTP 400 and should
be strengthened to also assert the error response JSON produced by
GlobalExceptionHandler: verify fields like $.status (equals 400), $.message
(contains validation error or specific message), and any $.errors array or
$.timestamp expected by the contract. Update the mockMvc assertions to include
jsonPath checks (e.g., .andExpect(jsonPath("$.status").value(400)) and
.andExpect(jsonPath("$.message").exists() / containsString(...))) so the test
verifies both status and error payload structure/contents.

---

34-34: Align ObjectMapper with other test classes for consistency.

Other controller tests in the codebase (CommentControllerTest, MedicalRecordControllerTest) inject ObjectMapper via @Autowired. Use the same pattern here for consistency and to ensure any custom Jackson configuration is automatically picked up if added in the future.

♻️ Proposed change

-    private final ObjectMapper objectMapper = new ObjectMapper();
+    `@Autowired`
+    private ObjectMapper objectMapper;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/test/java/org/example/vet1177/controller/VetControllerTest.java` at line
34, In VetControllerTest replace the manually constructed ObjectMapper field
with the same injected pattern used in other tests: remove "new ObjectMapper()"
and make the field an `@Autowired` ObjectMapper so the test class
(VetControllerTest) receives the shared Jackson configuration; locate the
private final ObjectMapper objectMapper declaration and change it to a
non-final, `@Autowired` ObjectMapper field to match CommentControllerTest and
MedicalRecordControllerTest.

---

68-69: Use nullable(User.class) instead of any(User.class) for null-principal matchers.

Since @AutoConfigureMockMvc(addFilters = false) disables security filters, the @AuthenticationPrincipal User user parameter will be null at runtime. In Mockito 5.20.x, any(Class<T>) does not match null; use nullable(User.class) to explicitly match the expected null argument.

♻️ Proposed change

-        doNothing().when(adminPolicy).requireAdmin(any(User.class));
+        doNothing().when(adminPolicy).requireAdmin(nullable(User.class));

@@
-        doThrow(new ForbiddenException("Forbidden"))
-                .when(adminPolicy)
-                .requireAdmin(any());
+        doThrow(new ForbiddenException("Forbidden"))
+                .when(adminPolicy)
+                .requireAdmin(nullable(User.class));

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/test/java/org/example/vet1177/controller/VetControllerTest.java` around
lines 68 - 69, The test uses
doNothing().when(adminPolicy).requireAdmin(any(User.class)) which fails to match
the actual null `@AuthenticationPrincipal` at runtime; replace any(User.class)
with nullable(User.class) in the requireAdmin stubbing so Mockito will match a
null principal (i.e., update the doNothing().when(adminPolicy).requireAdmin(...)
call to use nullable(User.class)); no other behavioral changes to
vetService.createVet(...) are required.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/test/java/org/example/vet1177/controller/VetControllerTest.java`:
- Around line 99-111: The tests (e.g., shouldReturn400_whenInvalidRequest and
the other negative-path test around lines 164-172) only assert HTTP 400 and
should be strengthened to also assert the error response JSON produced by
GlobalExceptionHandler: verify fields like $.status (equals 400), $.message
(contains validation error or specific message), and any $.errors array or
$.timestamp expected by the contract. Update the mockMvc assertions to include
jsonPath checks (e.g., .andExpect(jsonPath("$.status").value(400)) and
.andExpect(jsonPath("$.message").exists() / containsString(...))) so the test
verifies both status and error payload structure/contents.
- Line 34: In VetControllerTest replace the manually constructed ObjectMapper
field with the same injected pattern used in other tests: remove "new
ObjectMapper()" and make the field an `@Autowired` ObjectMapper so the test class
(VetControllerTest) receives the shared Jackson configuration; locate the
private final ObjectMapper objectMapper declaration and change it to a
non-final, `@Autowired` ObjectMapper field to match CommentControllerTest and
MedicalRecordControllerTest.
- Around line 68-69: The test uses
doNothing().when(adminPolicy).requireAdmin(any(User.class)) which fails to match
the actual null `@AuthenticationPrincipal` at runtime; replace any(User.class)
with nullable(User.class) in the requireAdmin stubbing so Mockito will match a
null principal (i.e., update the doNothing().when(adminPolicy).requireAdmin(...)
call to use nullable(User.class)); no other behavioral changes to
vetService.createVet(...) are required.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: fb04511e-b17a-460d-8bc3-07682d4a43c1

📥 Commits

Reviewing files that changed from the base of the PR and between 5a74e27 and a7316ac.

📒 Files selected for processing (2)

• pom.xml
• src/test/java/org/example/vet1177/controller/VetControllerTest.java