Clean up exception handling in StashApiClient

[proski]

The first commit adds WireMock based unit tests for the entire StashApiClient.java. In some cases, the tests indicate issues in the code under test. Those issues are documented.

The second commit changed exception handling in StashApiClient. Only specific checked exceptions are used, RuntimeException and Exception are banned. The callers are changes to handle the exceptions. That code was getting RuntimeException and ignored it. Now it's forces to deal with it properly.

[proski]

Improved tests for loading PRs and PR comments from multiple pages to avoid some repetition.

[jakub-bochenski]

I don't see a good way to embed JSON into Java code (https://openjdk.java.net/jeps/355 will hopefully change it), so I'm constructing JSON programmatically.

How about we use POJOs and then serialize them into Json with Jackson etc.? For simple objects (I don't expect anything else) the transformation should be obvious.

Another idea would be to go for spring-cloud-contract, too bad Stash doesn't publish it's API contracts (or does it?)

[proski]

How about we use POJOs and then serialize them into Json with Jackson etc.? For simple objects (I don't expect anything else) the transformation should be obvious.

I don't think is would be simpler than what we have now.

Another idea would be to go for spring-cloud-contract, too bad Stash doesn't publish it's API contracts (or does it?)

A quick Google search suggests that they exist, but no indication where they are. Again, I don't expect it to be a simplification.

I'm going to test standalone JSON files first.

[proski]

Added support for all public methods except deleteRequest(), which is public by mistake. Still need to test more exceptional conditions like connection reset. Need to test responses better, especially for POST.

It turns out WireMock has good support for configuring through JSON files, I'm going to try it.

[proski]

Converted multi-page tests to JSON files.

Using dynamic port for HTTP. There default is port 8080, which can conflict with Jenkins.

Using wiremock-jre8-standalone in pom.xml, it's recommended if Java 8+ is required.

[proski]

This PR has achieved 91.9% code coverage for StashApiClient.java, which is pretty impressive. Not only is the code under test executed, the outputs are checked.

One problem is that only "easy HTTPS" is being tested, but not HTTP or real HTTPS. I believe JUnit has a way to run tests with multiple parameters, I'll look into that. In any case, unit testing real HTTPS can be tricky.

The unit tests have discovered multiple issues. I documented them in the code.

• RuntimeException is thrown, but the callers may not be prepared for it, as it's not a checked exception
• Many calls ignore HTTP errors, especially "malformed response"
• deletePullRequestComment() gives no indication whether the call has succeeded
• mergePullRequest() throws on 409 Conflict instead of returning false as apparently intended
• There are no checks whether the HTTP code indicates an error for the specific request
• POST requests throw on 204 No Content
• Timeouts are not testable without very slow tests, as the timings are not configurable

Applying the first commit from #68 on top of this PR (i.e. upgrading the HTTP client) breaks several tests. On the positive side, some errors are not ignored anymore. On the negative side, RuntimeException is thrown in some cases where StashApiException was thrown.

I think the best course of action would be to completely remove RuntimeException from the code first. That will give us visibility into what exceptions we should handle.

[proski]

I've added a commit to clean up exception handling. It would be better to update the HTTP client if we have a complete picture which exceptions are thrown and what code is handling them.

I didn't try to use perfect messages in the exceptions, I only wanted then to be better than what it is now. I'm trying to keep this PR short so it can be reviewed and merged quickly.

[proski]

It turns out I removed stack traces in many places, please don't merge this PR until I fix that issue.

[proski]

Should be good to merge now.

[proski]

I added comments everywhere StashApiException is handled. I also added javadoc comments to the functions that throw StashApiException in StashRepository.

[proski]

Added unit tests for StashBuildListener.

[proski]

Added unit tests for the changes to StashRepository.java.

[proski]

Added checks for authentication, we really don't want to break it.