Skip to content

Commit

Permalink
Class design changes for GitLabApiTransport
Browse files Browse the repository at this point in the history
* get and post should return Optionals, as the GsonParserUtil.parseHttpResponse method can return a null.
* replaceClazz is replaced with responseType, which is a Type. This lets us pass in parameterized Types for GSON to case responses to.

BUG=393370224
PiperOrigin-RevId: 726574202
Change-Id: I82694eca30763fe70494e948755207e034d21482
  • Loading branch information
chris-campos authored and copybara-github committed Feb 14, 2025
1 parent 1830e2a commit 9230fff
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 24 deletions.
14 changes: 9 additions & 5 deletions java/com/google/copybara/git/gitlab/api/GitLabApiTransport.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
import com.google.copybara.exception.RepoException;
import com.google.copybara.exception.ValidationException;
import com.google.copybara.git.gitlab.api.entities.GitLabApiEntity;
import java.lang.reflect.Type;
import java.util.Optional;

/** An interface for transports that communicate with a GitLab API endpoint. */
public interface GitLabApiTransport {
Expand All @@ -29,12 +31,13 @@ public interface GitLabApiTransport {
*
* @param <T> the class the JSON response will be parsed to
* @param path the path to call, e.g. projects/13422/merge_requests
* @param responseType the Java type that GSON should parse the response into
* @param headers the headers to add to the HTTP request
* @return the returned {@link T}
* @return the returned {@link T}, if a response is returned
* @throws GitLabApiException if there is an issue performing the request
* @throws ValidationException if there is an issue with credential issuing or retrieval
*/
<T> T get(String path, Class<T> responseClazz, ImmutableListMultimap<String, String> headers)
<T> Optional<T> get(String path, Type responseType, ImmutableListMultimap<String, String> headers)
throws GitLabApiException, ValidationException;

/**
Expand All @@ -43,14 +46,15 @@ <T> T get(String path, Class<T> responseClazz, ImmutableListMultimap<String, Str
* @param <T> the class the JSON response will be parsed to
* @param path the path to call, e.g. projects/13422/merge_requests
* @param request the object to send as part of the request
* @return the returned {@link T}
* @param responseType the Java type that GSON should parse the response into
* @return the returned {@link T}, if a response is returned
* @throws GitLabApiException if there is an issue performing the request
* @throws ValidationException if there is an issue with credential issuing or retrieval
*/
<T> T post(
<T> Optional<T> post(
String path,
GitLabApiEntity request,
Class<T> responseClazz,
Type responseType,
ImmutableListMultimap<String, String> headers)
throws RepoException, ValidationException;

Expand Down
20 changes: 10 additions & 10 deletions java/com/google/copybara/git/gitlab/api/GitLabApiTransportImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,10 @@
import com.google.copybara.http.auth.AuthInterceptor;
import com.google.copybara.json.GsonParserUtil;
import com.google.copybara.util.console.Console;
import com.google.gson.reflect.TypeToken;
import java.io.IOException;
import java.lang.reflect.Type;
import java.net.URI;
import java.util.Optional;
import javax.annotation.Nullable;

/**
Expand Down Expand Up @@ -64,16 +65,16 @@ public GitLabApiTransportImpl(
}

@Override
public <T> T get(
String path, Class<T> responseClazz, ImmutableListMultimap<String, String> headers)
public <T> Optional<T> get(
String path, Type responseType, ImmutableListMultimap<String, String> headers)
throws GitLabApiException, ValidationException {
GenericUrl url = getFullEndpointGenericUrl(path);
try {
console.verboseFmt("Sending GET request to %s", url);
HttpResponse httpResponse = getGetHttpResponse(headers, url);
// TODO: b/394914756 - Support GitLab API pagination
return GsonParserUtil.parseHttpResponse(
httpResponse, TypeToken.get(responseClazz).getType(), false);
return Optional.ofNullable(
GsonParserUtil.parseHttpResponse(httpResponse, responseType, false));
} catch (HttpResponseException e) {
throw new GitLabApiException(
String.format("Error calling GET on %s", url), e.getStatusCode(), e);
Expand Down Expand Up @@ -107,19 +108,18 @@ private HttpRequest getHttpRequest(
}

@Override
public <T> T post(
public <T> Optional<T> post(
String path,
GitLabApiEntity request,
Class<T> responseClazz,
Type responseType,
ImmutableListMultimap<String, String> headers)
throws GitLabApiException, ValidationException {
GenericUrl url = getFullEndpointGenericUrl(path);
try {
console.verboseFmt("Sending POST request to %s", url);
HttpResponse httpResponse = getPostHttpResponse(request, url);
// TODO: b/394914756 - Support GitLab API pagination
return GsonParserUtil.parseHttpResponse(
httpResponse, TypeToken.get(responseClazz).getType(), false);
return Optional.ofNullable(
GsonParserUtil.parseHttpResponse(httpResponse, responseType, false));
} catch (HttpResponseException e) {
throw new GitLabApiException(
String.format("Error calling POST on %s", url), e.getStatusCode(), e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand Down Expand Up @@ -75,12 +76,13 @@ public void testSimpleGetRequest() throws Exception {
"https://gitlab.copybara.io/capybara/project",
bearerInterceptor);

TestResponse testResponse =
Optional<TestResponse> testResponse =
underTest.get(
"/projects/12345/test_request", TestResponse.class, ImmutableListMultimap.of());

assertThat(testResponse.getId()).isEqualTo(12345);
assertThat(testResponse.getDescription()).isEqualTo("capybara");
assertThat(testResponse).isPresent();
assertThat(testResponse.get().getId()).isEqualTo(12345);
assertThat(testResponse.get().getDescription()).isEqualTo("capybara");
}

@Test
Expand All @@ -95,7 +97,7 @@ public void testSimpleGetRequest_withHttpPort() throws Exception {
"https://gitlab.copybara.io:8080/capybara/project",
bearerInterceptor);

TestResponse unused =
Optional<TestResponse> unused =
underTest.get(
"/projects/12345/test_request", TestResponse.class, ImmutableListMultimap.of());

Expand Down Expand Up @@ -149,15 +151,16 @@ public void testSimplePostRequest() throws Exception {
bearerInterceptor);
TestRequest testRequest = new TestRequest(54321, "baracopy");

TestResponse testResponse =
Optional<TestResponse> testResponse =
underTest.post(
"/projects/12345/test_request",
testRequest,
TestResponse.class,
ImmutableListMultimap.of());

assertThat(testResponse.getId()).isEqualTo(12345);
assertThat(testResponse.getDescription()).isEqualTo("capybara");
assertThat(testResponse).isPresent();
assertThat(testResponse.get().getId()).isEqualTo(12345);
assertThat(testResponse.get().getDescription()).isEqualTo("capybara");
}

@Test
Expand All @@ -173,7 +176,7 @@ public void testSimplePostRequest_httpPort() throws Exception {
bearerInterceptor);
TestRequest testRequest = new TestRequest(54321, "baracopy");

TestResponse unused =
Optional<TestResponse> unused =
underTest.post(
"/projects/12345/test_request",
testRequest,
Expand Down Expand Up @@ -244,7 +247,7 @@ public void testSetPasswordHeaders() throws Exception {
getApiTransport(
httpTransport, "https://gitlab.copybara.io/capybara/project", bearerInterceptor);

TestResponse unused =
Optional<TestResponse> unused =
underTest.get(
"/projects/12345/test_request", TestResponse.class, ImmutableListMultimap.of());

Expand Down

0 comments on commit 9230fff

Please sign in to comment.