From d91acacf0bc43037dbb755cd455540d86405fac5 Mon Sep 17 00:00:00 2001 From: Min Zhu Date: Tue, 7 May 2024 03:55:08 +0000 Subject: [PATCH 1/3] feat: throw exception when user attempt to override api-version header. --- .../api/gax/rpc/ApiClientHeaderProvider.java | 2 +- .../com/google/api/gax/rpc/ClientContext.java | 7 ++ .../google/api/gax/rpc/ClientContextTest.java | 105 ++++++++++++++++++ 3 files changed, 113 insertions(+), 1 deletion(-) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ApiClientHeaderProvider.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ApiClientHeaderProvider.java index ae27404335..a5e80e10b1 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ApiClientHeaderProvider.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ApiClientHeaderProvider.java @@ -42,7 +42,7 @@ public class ApiClientHeaderProvider implements HeaderProvider, Serializable { private static final long serialVersionUID = -8876627296793342119L; static final String QUOTA_PROJECT_ID_HEADER_KEY = "x-goog-user-project"; - static final String API_VERSION_HEADER_KEY = "x-goog-api-version"; + public static final String API_VERSION_HEADER_KEY = "x-goog-api-version"; private final Map headers; diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java index 26cf63eb85..2c5a2ec339 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java @@ -290,6 +290,13 @@ private static Map getHeadersFromSettings(StubSettings settings) Map internalHeaders = settings.getInternalHeaderProvider().getHeaders(); Map conflictResolution = new HashMap<>(); + // api version header user override is not allowed, even when not set by internal headers + if (userHeaders.keySet().contains(ApiClientHeaderProvider.API_VERSION_HEADER_KEY)) { + throw new IllegalArgumentException( + "Header provider can't override the header: " + + ApiClientHeaderProvider.API_VERSION_HEADER_KEY); + } + Set conflicts = Sets.intersection(userHeaders.keySet(), internalHeaders.keySet()); for (String key : conflicts) { if ("user-agent".equals(key)) { diff --git a/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java b/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java index 2f82489528..8a6e7c2c74 100644 --- a/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java +++ b/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java @@ -64,7 +64,9 @@ import java.util.concurrent.Executor; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledThreadPoolExecutor; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; import org.mockito.Mockito; @@ -681,6 +683,109 @@ public void testUserAgentConcat() throws Exception { .containsEntry("user-agent", "user-supplied-agent internal-agent"); } + @Rule public ExpectedException apiVersionExceptionRule = ExpectedException.none(); + + @Test + public void testApiVersionHeaderConflictShouldThrowException() throws Exception { + + apiVersionExceptionRule.expect(IllegalArgumentException.class); + apiVersionExceptionRule.expectMessage( + "Header provider can't override the header: " + + ApiClientHeaderProvider.API_VERSION_HEADER_KEY); + + TransportChannelProvider transportChannelProvider = + new FakeTransportProvider( + FakeTransportChannel.create(new FakeChannel()), + null, + true, + null, + null, + DEFAULT_ENDPOINT); + + ClientSettings.Builder builder = + new FakeClientSettings.Builder() + .setExecutorProvider( + FixedExecutorProvider.create(Mockito.mock(ScheduledExecutorService.class))) + .setTransportChannelProvider(transportChannelProvider) + .setCredentialsProvider( + FixedCredentialsProvider.create(Mockito.mock(GoogleCredentials.class))); + + builder.setHeaderProvider( + FixedHeaderProvider.create( + ApiClientHeaderProvider.API_VERSION_HEADER_KEY, "user-supplied-version")); + builder.setInternalHeaderProvider( + FixedHeaderProvider.create( + ApiClientHeaderProvider.API_VERSION_HEADER_KEY, "internal-version")); + + ClientContext.create(builder.build()); + } + + @Test + public void testApiVersionHeaderEmptyConflictShouldThrowException() throws Exception { + + apiVersionExceptionRule.expect(IllegalArgumentException.class); + apiVersionExceptionRule.expectMessage( + "Header provider can't override the header: " + + ApiClientHeaderProvider.API_VERSION_HEADER_KEY); + + TransportChannelProvider transportChannelProvider = + new FakeTransportProvider( + FakeTransportChannel.create(new FakeChannel()), + null, + true, + null, + null, + DEFAULT_ENDPOINT); + + ClientSettings.Builder builder = + new FakeClientSettings.Builder() + .setExecutorProvider( + FixedExecutorProvider.create(Mockito.mock(ScheduledExecutorService.class))) + .setTransportChannelProvider(transportChannelProvider) + .setCredentialsProvider( + FixedCredentialsProvider.create(Mockito.mock(GoogleCredentials.class))); + + builder.setHeaderProvider( + FixedHeaderProvider.create( + ApiClientHeaderProvider.API_VERSION_HEADER_KEY, "user-supplied-version")); + builder.setInternalHeaderProvider( + FixedHeaderProvider.create(ApiClientHeaderProvider.API_VERSION_HEADER_KEY, "")); + + ClientContext.create(builder.build()); + } + + @Test + public void testUserApiVersionHeaderOnlyShouldThrowException() throws Exception { + + apiVersionExceptionRule.expect(IllegalArgumentException.class); + apiVersionExceptionRule.expectMessage( + "Header provider can't override the header: " + + ApiClientHeaderProvider.API_VERSION_HEADER_KEY); + + TransportChannelProvider transportChannelProvider = + new FakeTransportProvider( + FakeTransportChannel.create(new FakeChannel()), + null, + true, + null, + null, + DEFAULT_ENDPOINT); + + ClientSettings.Builder builder = + new FakeClientSettings.Builder() + .setExecutorProvider( + FixedExecutorProvider.create(Mockito.mock(ScheduledExecutorService.class))) + .setTransportChannelProvider(transportChannelProvider) + .setCredentialsProvider( + FixedCredentialsProvider.create(Mockito.mock(GoogleCredentials.class))); + + builder.setHeaderProvider( + FixedHeaderProvider.create( + ApiClientHeaderProvider.API_VERSION_HEADER_KEY, "user-supplied-version")); + + ClientContext.create(builder.build()); + } + private static String endpoint = "https://foo.googleapis.com"; private static String mtlsEndpoint = "https://foo.mtls.googleapis.com"; From d291dc786639cf72f652c7f55913d864b41aba7e Mon Sep 17 00:00:00 2001 From: Min Zhu Date: Tue, 7 May 2024 04:01:52 +0000 Subject: [PATCH 2/3] extract helper method. --- .../google/api/gax/rpc/ClientContextTest.java | 73 +++++-------------- 1 file changed, 18 insertions(+), 55 deletions(-) diff --git a/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java b/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java index 8a6e7c2c74..dcaff97503 100644 --- a/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java +++ b/gax-java/gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java @@ -685,14 +685,8 @@ public void testUserAgentConcat() throws Exception { @Rule public ExpectedException apiVersionExceptionRule = ExpectedException.none(); - @Test - public void testApiVersionHeaderConflictShouldThrowException() throws Exception { - - apiVersionExceptionRule.expect(IllegalArgumentException.class); - apiVersionExceptionRule.expectMessage( - "Header provider can't override the header: " - + ApiClientHeaderProvider.API_VERSION_HEADER_KEY); - + private void createClientContextAndSetApiVersionHeaders( + String internalVersion, String userVersion) throws IOException { TransportChannelProvider transportChannelProvider = new FakeTransportProvider( FakeTransportChannel.create(new FakeChannel()), @@ -713,45 +707,35 @@ public void testApiVersionHeaderConflictShouldThrowException() throws Exception builder.setHeaderProvider( FixedHeaderProvider.create( ApiClientHeaderProvider.API_VERSION_HEADER_KEY, "user-supplied-version")); - builder.setInternalHeaderProvider( - FixedHeaderProvider.create( - ApiClientHeaderProvider.API_VERSION_HEADER_KEY, "internal-version")); + if (internalVersion != null) { + builder.setInternalHeaderProvider( + FixedHeaderProvider.create( + ApiClientHeaderProvider.API_VERSION_HEADER_KEY, "internal-version")); + } ClientContext.create(builder.build()); } @Test - public void testApiVersionHeaderEmptyConflictShouldThrowException() throws Exception { + public void testApiVersionHeaderConflictShouldThrowException() throws Exception { apiVersionExceptionRule.expect(IllegalArgumentException.class); apiVersionExceptionRule.expectMessage( "Header provider can't override the header: " + ApiClientHeaderProvider.API_VERSION_HEADER_KEY); - TransportChannelProvider transportChannelProvider = - new FakeTransportProvider( - FakeTransportChannel.create(new FakeChannel()), - null, - true, - null, - null, - DEFAULT_ENDPOINT); + createClientContextAndSetApiVersionHeaders("internal-version", "user-supplied-version"); + } - ClientSettings.Builder builder = - new FakeClientSettings.Builder() - .setExecutorProvider( - FixedExecutorProvider.create(Mockito.mock(ScheduledExecutorService.class))) - .setTransportChannelProvider(transportChannelProvider) - .setCredentialsProvider( - FixedCredentialsProvider.create(Mockito.mock(GoogleCredentials.class))); + @Test + public void testApiVersionHeaderEmptyConflictShouldThrowException() throws Exception { - builder.setHeaderProvider( - FixedHeaderProvider.create( - ApiClientHeaderProvider.API_VERSION_HEADER_KEY, "user-supplied-version")); - builder.setInternalHeaderProvider( - FixedHeaderProvider.create(ApiClientHeaderProvider.API_VERSION_HEADER_KEY, "")); + apiVersionExceptionRule.expect(IllegalArgumentException.class); + apiVersionExceptionRule.expectMessage( + "Header provider can't override the header: " + + ApiClientHeaderProvider.API_VERSION_HEADER_KEY); - ClientContext.create(builder.build()); + createClientContextAndSetApiVersionHeaders("", "user-supplied-version"); } @Test @@ -762,28 +746,7 @@ public void testUserApiVersionHeaderOnlyShouldThrowException() throws Exception "Header provider can't override the header: " + ApiClientHeaderProvider.API_VERSION_HEADER_KEY); - TransportChannelProvider transportChannelProvider = - new FakeTransportProvider( - FakeTransportChannel.create(new FakeChannel()), - null, - true, - null, - null, - DEFAULT_ENDPOINT); - - ClientSettings.Builder builder = - new FakeClientSettings.Builder() - .setExecutorProvider( - FixedExecutorProvider.create(Mockito.mock(ScheduledExecutorService.class))) - .setTransportChannelProvider(transportChannelProvider) - .setCredentialsProvider( - FixedCredentialsProvider.create(Mockito.mock(GoogleCredentials.class))); - - builder.setHeaderProvider( - FixedHeaderProvider.create( - ApiClientHeaderProvider.API_VERSION_HEADER_KEY, "user-supplied-version")); - - ClientContext.create(builder.build()); + createClientContextAndSetApiVersionHeaders(null, "user-supplied-version"); } private static String endpoint = "https://foo.googleapis.com"; From 8a04cffc47407b8cba0c4ca6f86d3fa4fe976aa3 Mon Sep 17 00:00:00 2001 From: Min Zhu Date: Tue, 7 May 2024 06:11:47 +0000 Subject: [PATCH 3/3] revert unnecessary modifier change for this change. --- .../java/com/google/api/gax/rpc/ApiClientHeaderProvider.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ApiClientHeaderProvider.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ApiClientHeaderProvider.java index a5e80e10b1..ae27404335 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ApiClientHeaderProvider.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ApiClientHeaderProvider.java @@ -42,7 +42,7 @@ public class ApiClientHeaderProvider implements HeaderProvider, Serializable { private static final long serialVersionUID = -8876627296793342119L; static final String QUOTA_PROJECT_ID_HEADER_KEY = "x-goog-user-project"; - public static final String API_VERSION_HEADER_KEY = "x-goog-api-version"; + static final String API_VERSION_HEADER_KEY = "x-goog-api-version"; private final Map headers;