RFR: 1092: /backport commit command does not validate non existing repos

[Kevin Rushforth]

On Wed, 23 Jun 2021 21:09:46 GMT, Erik Joelsson <erikj at openjdk.org> wrote:

> This patch fixes inconsistencies with how Forge.repository(String) behaves for GitHub and GitLab. 
> 
> From what I can tell, this method is expected to return an Optional.empty() if the repository does not exist, and in the Gitlab case, this is what it does. For Github there is no actual check. In both cases, they call the hosted repository constructor (GitlabRepository and GithubRepository respectively) and expect an exception if it doesn't exist. In the case of Gitlab, the constructor will make a rest call to get information about the repository, while the Github variant does not. It seems this was changed for GitHub a long while back as an optimization, using lazy initialization: https://github.com/openjdk/skara/pull/296
> 
> I have rewritten this logic and aligned the behavior of Forge.repository(String) between GitHub and GitLab. Now they both will only return Optional.empty() if the rest call to get repository info returns 404. Any other RuntimeException or Error is just let through, as it should be. There are other ways a hosted repository can be created, where Optional is not used in the return value. In these cases we now have consistent throwing of a RuntimeException with the message "Project/Repository not found" (in the GitLab case directly as it's needed for constructing the object, and in the GitHub case when a method is called that triggers the lazy initialization). The assumption is that in these other code paths, the repository name does not originate from human input, but rather other query results, so a 404 is truly unexpected and should result in an exception.

Looks fine. Do you think it is worth adding additional tests to make sure a 404 correctly returns an empty optional, whereas any other error throws an exception?

-------------

Marked as reviewed by kcr (Reviewer).

PR: https://git.openjdk.java.net/skara/pull/1192