Skip to content
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

TResource.Delete() method change request. #136

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

IgorKaplya
Copy link

Hi, @fabriciocolombo and team. Thank you for your work. I'm Igor and I'm a Delphi dev. Currently I'm working on API integration which returns content on DELETE method.
This PR:

  • adds possibility to handle API response for Delete.
  • fixes situation when http errors (for example 404) don't trigger Client OnError.
  • updates java-rest-server controllers for proper working with unit tests.

Because I'm assuming absense of "@produces" on DELETE methods in controllers is a bad server side design. It should not be hacked on client side as we did.

Please consider PR #135. It fixes java-rest-server compilation for JDK 1.8.

Igor Kaplya and others added 3 commits March 13, 2020 20:40
 * Added result to TResource.Delete() method.  API's are free to return it. Mine does.
 * Indy delete method works fine. No need to hack it.
 * This patch fixes situation when HTTP errors from Indy don't trigger TRestClient.OnError.
Added "@produces" for Delete method. This fixes fail of "DeleteJsonObject" and "RemovePerson" unit tests.
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.

1 participant