Skip to content

Fix #287 hide technical exception messages in API error responses (SQL / Hibernate details)#302

Merged
vfedoriv merged 3 commits intospring-petclinic:masterfrom
San-43:fix-287-hide-technical-error-details
Mar 12, 2026
Merged

Fix #287 hide technical exception messages in API error responses (SQL / Hibernate details)#302
vfedoriv merged 3 commits intospring-petclinic:masterfrom
San-43:fix-287-hide-technical-error-details

Conversation

@San-43
Copy link
Copy Markdown
Contributor

@San-43 San-43 commented Mar 6, 2026

This PR fixes issue #287.

I reproduced the issue locally, added tests, and implemented a fix.

Changes included:

  • validate owner existence before saving a pet.
  • avoid exposing technical persistence details in API responses.
  • add tests for the missing-owner scenario and error response sanitization.
  • update testCreatePetSuccess to match the new logic.

Please, let me know if there is anything wrong.

@San-43 San-43 changed the title Fix #287 hide technical persistence details in pet creation errors Fix #287 hide technical exception messages in API error responses (SQL / Hibernate details) Mar 10, 2026
import java.util.List;

import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.not;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@San-43 can you use AssertJ equivalents instead, pls?
we already have some mix of styles/test libraries, it will be good to stay with AssertJ by default.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @vfedoriv, thank you for your review, I'll be working on it.

Copy link
Copy Markdown
Contributor Author

@San-43 San-43 Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@San-43 can you use AssertJ equivalents instead, pls? we already have some mix of styles/test libraries, it will be good to stay with AssertJ by default.

Hi @vfedoriv, I updated the test to use AssertJ equivalents, removed unnecessary imports, and adjusted the logic as well, since I believe the previous version was not quite correct. Pls let me know if there's anything else that i should change.

@San-43 San-43 force-pushed the fix-287-hide-technical-error-details branch from ef8ac91 to 407f7ca Compare March 11, 2026 22:34
@vfedoriv vfedoriv merged commit c2cdc05 into spring-petclinic:master Mar 12, 2026
2 checks passed
@San-43
Copy link
Copy Markdown
Contributor Author

San-43 commented Mar 12, 2026

Thank you very much @vfedoriv for your review and for merging. It means a lot to me.

@San-43 San-43 deleted the fix-287-hide-technical-error-details branch March 12, 2026 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants