getOptionalXXX() method can return false Optional.empty()

[amitt00r]

trafficstars

We recently faced one issue where below method returned false Optional.empty() for a file which did exist. On checking the code found issue with below code in RepositoryFileApi class.

  public Optional<RepositoryFile> getOptionalFileInfo(Object projectIdOrPath, String filePath, String ref) {
        try {
            return Optional.ofNullable(this.getFileInfo(projectIdOrPath, filePath, ref));
        } catch (GitLabApiException var5) {
            return GitLabApi.createOptionalFromException(var5);
        }
    }

Issue: Method GitLabApi.createOptionalFromException(var5) above doesn't do status code checks on GitLabApiException before returning Optional.empty(). This method is used across all getOptionalXXX() methods in all Apis and this means all getOptionalXXX() methods in all repositories has this issue.

Expected Behaviour: This method should first check on 'httpStatus' == 404 to ensure that file indeed doesn't exist on server and then return optional.empty(). All other exceptions should be rethrown wrapped as Suitable Exception.

[jmini]

Interesting finding.

The problem I see with "rethrow": all the getOptionalXXX() methods are not throwing any Exception. So we should maybe wrap it into a Runtime exception. Not sure if it makes sense or not.

I looked at the code and I am also not convinced by the mechanism of storing the exception in a map.

[amitt00r]

Yep, I found it later all methods returning Optional have this issue, all use same method GitlabApi.createOptionalFromException() which never throws exception no matter what the status code is.