chore: removal of Credential argument in newApi() methods

[diegomarquezp]

No description provided.

[burkedavison]

Preconditions.checkState --> "Ensures the truth of an expression involving the state of the calling instance, but not involving any parameters to the calling method."

Preconditions.checkArgument --> "Ensures the truth of an expression involving one or more parameters to the calling method."

From https://guava.dev/releases/19.0/api/docs/com/google/common/base/Preconditions.html

[burkedavison]

@diegomarquezp : [Paraphrased] Should we throw these IOExceptions when unable to get the credential?

Generally, it depends on whether the IOException simply represents "no credential", or whether it is truly "exceptional" in the sense that "something unexpected went wrong". Also, generally, it depends on whether the caller of the function would potentially be capable of handling / recovering from the exception in some way that the current method cannot.

I would generally expect a boolean-returning method like hasCredentialsSet to return false if the IOException is simply getOAuth2Credential()'s standard behavior for saying "no credential".

@diegomarquezp : [Paraphrased] Should we return null from getCredential()?

I've jumped on the Java 8 train, and returning null is against my religion. Instead, I would be returning an never-null Optional<Credential> reference. However, the rest of this code base tends to return null often, so there is an argument to be made that maintaining the existing standard is okay.

Ultimately these are trade-offs that depend on the context and how heavily you weigh each consideration. Given the above, what do you think is the best choice in this situation that lets you achieve the desired goal?

[diegomarquezp]

In that sense, the IOException occurs when something goes wrong when obtaining the user info via an HTTP request using the default credentials as base. It is therefore truly exceptional. I will modify this to bubble up the exception.

[diegomarquezp]

Actually, I think it's fine to bubble it up to GoogleApiFactory where then the exception is catched, but there is still a null check for the credentials in every new[...]Api() method. I think it's fair to leave external callers unconcerned with a failed internal HTTP request to obtain user info.

[burkedavison]

+ ex is effectively + ex.toString() which will print the exception class name and maybe an instance ID. If you're at the point of logging an exception, always do it in a way that includes the stack trace.

LOGGER.log(Level.SEVERE, "Error when obtaining credential.", ex);