[PT-694] Do not throw exception for successful created http code

[Harmannz]

The content-watch service returns 201 upon creation of a new watch.

[dylannz-sailthru]

Looks OK to me, may want review from NY or SRE team

[dylannz-sailthru]

nit:

Suggested change

	CloseableHttpResponse httCreatedResponse = getMockHttpResponseWithStatus(HttpStatus.SC_INTERNAL_SERVER_ERROR, "Internal Server Error");
	try {
	Object okResponse = handler.handleResponse(httCreatedResponse);
	Assert.fail("Expected an APIException to be thrown");
	CloseableHttpResponse httpCreatedResponse = getMockHttpResponseWithStatus(HttpStatus.SC_INTERNAL_SERVER_ERROR, "Internal Server Error");
	try {
	Object okResponse = handler.handleResponse(httpCreatedResponse);
	Assert.fail("Expected an APIException to be thrown");

[Harmannz]

ahh thats terrible copy past from me. brb with a fix

[m1dcarry]

LGTM

[Harmannz]

Force pushed to update variable names for test method

[kryptx]

Possibly I misunderstand something, but I think there are a couple more cases to handle.

[kryptx]

Is there a reason we didn't also add 204 (this is returned by the DELETE endpoint)? Also the POST endpoint will return 409 if the watch already exists, it seems like that might be useful for a client to know.

[Harmannz]

Hey thanks for the feedback. I was planning to release this incrementally mainly to derisk my changes. But I think its easy for me to include the 202 and 204 code here.
409 and other http codes will still be thrown back to upstream via the default switch case.

[kryptx]

Aye, I was thinking the switch case below would just return some kind of generic error, but you're right, they can already pull the 409 status code out of that with no further changes.

[kryptx]

As for 202, I cannot see a reason the client should throw if we receive it, but if you still want to follow-up with a refactor proposal then I'd say it could be left out of this PR. We currently will never return 202 from PHP API.

[Harmannz]

Alright. So just need to add 204 code for the DELETE endpoint.

[mattdudys]

A style comment about unit testing expected exceptions. The main change of categorizing 201 and 204 looks good to me, +1.

[mattdudys]

Using ExceptionRule can help simplify these try-execute-fail-catch-assert patterns. https://junit.org/junit4/javadoc/4.12/org/junit/rules/ExpectedException.html

This looks like two tests in one method. Can you split them apart more? If you were to use ExceptionRule you would want to just test one response at a time.