fix: fix buildRequest setUrl order

[arithmetic1728]

request.setUrl should be called before initializer.intialize(request), otherwise program may crash if request has a null uri.

For example, HttpCredentialsAdapter is a common implementation of HttpRequestInitializer. In its initialize(request) implementation, it calls Map<String, List<String>> credentialHeaders = credentials.getRequestMetadata(uri);.

Note if the credentials is a ServiceAccountJWTAccessCredentials, then getRequestMetadata(uri) throws exception if both uri and defaultAudience are both null. Moreover, most times uri is the audience used for jwt token, it should be passed instead of being null all the time.

[elharo]

test?

[arithmetic1728]

test?

done.

[arithmetic1728]

@elharo could you take another look? This fix will be needed for self signed jwt feature. Thanks!

[elharo]

tests are failing. Might not be related but should be fixed first

[arithmetic1728]

tests are failing. Might not be related but should be fixed first

The failures are not related to my changes. They already failed in the recent submitted PRs. There were two failures: one is YouTube sample style check error, the other is Jackson 2 package javadoc error.

I fixed the style check failure in the YouTube sample by running mvn com.coveo:fmt-maven-plugin:format and changing the license url (https->http). Now the error becomes enforcer errors, I don't have much experience with Java, not sure how to fix this kind of problem. I think probably we can check in this PR, I will file a bug to let the Java experts fix it.

[WARNING] Rule 2: org.apache.maven.plugins.enforcer.RequireUpperBoundDeps failed with message:
Failed while enforcing RequireUpperBoundDeps. The error(s) are [
Require upper bound dependencies error for com.google.errorprone:error_prone_annotations:2.3.4 paths to dependency are:
+-com.google.http-client:google-http-client:1.38.2-SNAPSHOT
  +-com.google.guava:guava:30.1-android
    +-com.google.errorprone:error_prone_annotations:2.3.4
and
+-com.google.http-client:google-http-client:1.38.2-SNAPSHOT
  +-com.google.guava:guava-testlib:30.1-android
    +-com.google.errorprone:error_prone_annotations:2.3.4
and
+-com.google.http-client:google-http-client:1.38.2-SNAPSHOT
  +-com.google.truth:truth:1.1.2
    +-com.google.errorprone:error_prone_annotations:2.5.1
]

[elharo]

Try just fixing the youtube samples in a separate PR. This is #1253 It's not related but it does need to be fixed before anything else can proceed.

[chingor13]

This is potentially a breaking behavior change for someone if their request initializer is conditionally setting the url (which would be a slightly odd check because it would never be previously set).

The existing test cases do not guarantee this to be true, though.

[arithmetic1728]

This is potentially a breaking behavior change for someone if their request initializer is conditionally setting the url (which would be a slightly odd check because it would never be previously set).

The existing test cases do not guarantee this to be true, though.

If their request initializer sets the url, then in the original code the url in the request will be overwritten, so it should be fine.

if (url != null) {
      request.setUrl(url);
    }

[chingor13]

This is potentially a breaking behavior change for someone if their request initializer is conditionally setting the url (which would be a slightly odd check because it would never be previously set).
The existing test cases do not guarantee this to be true, though.

If their request initializer sets the url, then in the original code the url in the request will be overwritten, so it should be fine.

if (url != null) {
      request.setUrl(url);
    }

The edge case I'm thinking of is if their initializer conditionally sets the url if not set. In the current implementation, the initializer would have set it, but now if it's set earlier, their initializer wouldn't set it. Granted this is a strange usage of the initializer.

[arithmetic1728]

@elharo can we merge this PR? Thanks!

[chingor13]

I'm no long concerned with a RequestInitializer doing something like:

if (request.getUrl() != null) {
  request.setUrl("some override");
}

Previously, request.getUrl() would always have been null, so they would have used an unconditional request.setUrl("some override").