Skip to content

Commit 480d895

Browse files
gae-java-botgae-java-bot
committed
Rename env variables used to configure images bundled service modernization
PiperOrigin-RevId: 964424875 Change-Id: I893e63957176f473072179e2cdc699d30fc99892
1 parent 22440d6 commit 480d895

6 files changed

Lines changed: 81 additions & 44 deletions

File tree

‎api/src/main/java/com/google/appengine/api/images/GrpcImagesClient.java‎

Lines changed: 29 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -13,21 +13,23 @@
1313
// limitations under the License.
1414
package com.google.appengine.api.images;
1515

16+
import static com.google.common.base.Preconditions.checkState;
17+
import static com.google.common.base.Strings.isNullOrEmpty;
18+
import static java.util.concurrent.TimeUnit.SECONDS;
19+
1620
import com.google.appengine.api.EnvironmentProvider;
1721
import com.google.appengine.api.SystemEnvironmentProvider;
1822
import com.google.appengine.api.images.proto.ImagesServiceGrpc;
1923
import com.google.auth.oauth2.GoogleCredentials;
2024
import com.google.auth.oauth2.IdTokenCredentials;
2125
import com.google.auth.oauth2.IdTokenProvider;
2226
import com.google.common.annotations.VisibleForTesting;
23-
import static com.google.common.base.Strings.isNullOrEmpty;
2427
import io.grpc.CallCredentials;
2528
import io.grpc.ManagedChannel;
2629
import io.grpc.auth.MoreCallCredentials;
2730
import io.grpc.netty.shaded.io.grpc.netty.NettyChannelBuilder;
2831
import java.io.IOException;
2932
import java.net.URI;
30-
import static java.util.concurrent.TimeUnit.SECONDS;
3133
import java.net.URISyntaxException;
3234

3335
/** Client for interacting with the gRPC based Images service. */
@@ -72,34 +74,41 @@ private static GoogleCredentials getApplicationDefaultCredentials() {
7274
}
7375

7476
private String getTarget() {
75-
String endpoint = environmentProvider.getenv("IMAGES_SERVICE_ENDPOINT");
76-
if (isNullOrEmpty(endpoint)) {
77-
throw new IllegalStateException("IMAGES_SERVICE_ENDPOINT environment variable not set.");
78-
}
77+
String endpoint =
78+
environmentProvider.getenv(ImagesServiceFactoryImpl.IMAGES_SERVICE_ENDPOINT_ENV);
79+
checkState(
80+
!isNullOrEmpty(endpoint),
81+
ImagesServiceFactoryImpl.IMAGES_SERVICE_ENDPOINT_ENV + " environment variable not set.");
7982
try {
8083
URI uri = new URI(endpoint);
8184
String host = uri.getHost();
82-
if (host == null) {
83-
throw new IllegalStateException("Invalid URI in IMAGES_SERVICE_ENDPOINT: " + endpoint);
84-
}
85+
checkState(
86+
host != null,
87+
"Invalid URI in %s: %s",
88+
ImagesServiceFactoryImpl.IMAGES_SERVICE_ENDPOINT_ENV,
89+
endpoint);
8590
return host + ":443";
8691
} catch (URISyntaxException e) {
87-
throw new IllegalStateException("Invalid URI in IMAGES_SERVICE_ENDPOINT: " + endpoint, e);
92+
throw new IllegalStateException(
93+
"Invalid URI in "
94+
+ ImagesServiceFactoryImpl.IMAGES_SERVICE_ENDPOINT_ENV
95+
+ ": "
96+
+ endpoint,
97+
e);
8898
}
8999
}
90100

91101
private static CallCredentials createOidcCredentials(
92102
EnvironmentProvider environmentProvider, GoogleCredentials googleCredentials) {
93-
String endpoint = environmentProvider.getenv("IMAGES_SERVICE_ENDPOINT");
94-
if (isNullOrEmpty(endpoint)) {
95-
throw new IllegalStateException("IMAGES_SERVICE_ENDPOINT environment variable not set.");
96-
}
97-
98-
if (!(googleCredentials instanceof IdTokenProvider idTokenProvider)) {
99-
throw new IllegalStateException(
100-
"The Application Default Credentials do not support OIDC ID token generation.");
101-
}
102-
103+
String endpoint =
104+
environmentProvider.getenv(ImagesServiceFactoryImpl.IMAGES_SERVICE_ENDPOINT_ENV);
105+
checkState(
106+
!isNullOrEmpty(endpoint),
107+
ImagesServiceFactoryImpl.IMAGES_SERVICE_ENDPOINT_ENV + " environment variable not set.");
108+
checkState(
109+
googleCredentials instanceof IdTokenProvider,
110+
"The Application Default Credentials do not support OIDC ID token generation.");
111+
IdTokenProvider idTokenProvider = (IdTokenProvider) googleCredentials;
103112
IdTokenCredentials idTokenCredentials =
104113
IdTokenCredentials.newBuilder()
105114
.setTargetAudience(endpoint)

‎api/src/main/java/com/google/appengine/api/images/ImagesServiceFactoryImpl.java‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616

1717
package com.google.appengine.api.images;
1818

19+
import static com.google.common.base.Preconditions.checkArgument;
20+
1921
import com.google.appengine.api.EnvironmentProvider;
2022
import com.google.appengine.api.SystemEnvironmentProvider;
2123
import com.google.appengine.api.blobstore.BlobKey;
@@ -32,8 +34,10 @@
3234
*/
3335
final class ImagesServiceFactoryImpl implements IImagesServiceFactory {
3436

35-
@VisibleForTesting
36-
static final String USE_CUSTOM_IMAGES_GRPC_SERVICE_ENV = "USE_CUSTOM_IMAGES_GRPC_SERVICE";
37+
static final String USE_CUSTOM_IMAGES_GRPC_SERVICE_ENV =
38+
"APPENGINE_USE_CUSTOM_IMAGES_GRPC_SERVICE";
39+
40+
static final String IMAGES_SERVICE_ENDPOINT_ENV = "APPENGINE_IMAGES_SERVICE_ENDPOINT";
3741

3842
private EnvironmentProvider environmentProvider = new SystemEnvironmentProvider();
3943

@@ -67,9 +71,8 @@ public Image makeImageFromBlob(BlobKey blobKey) {
6771
@Override
6872
public Image makeImageFromFilename(String filename) {
6973
if (Boolean.parseBoolean(environmentProvider.getenv(USE_CUSTOM_IMAGES_GRPC_SERVICE_ENV))) {
70-
if (!filename.startsWith("/gs/")) {
71-
throw new IllegalArgumentException("Google storage filenames must be prefixed with /gs/");
72-
}
74+
checkArgument(
75+
filename.startsWith("/gs/"), "Google storage filenames must be prefixed with /gs/");
7376
return new ImageImpl(new BlobKey(filename));
7477
}
7578
BlobKey blobKey = BlobstoreServiceFactory.getBlobstoreService().createGsBlobKey(filename);

‎api/src/main/java/com/google/appengine/api/images/ImagesServiceImpl.java‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,8 @@ private ImagesServiceBlockingStub getGrpcStub() {
164164

165165
@VisibleForTesting
166166
boolean useGrpc() {
167-
String envVar = environmentProvider.getenv("USE_CUSTOM_IMAGES_GRPC_SERVICE");
167+
String envVar =
168+
environmentProvider.getenv(ImagesServiceFactoryImpl.USE_CUSTOM_IMAGES_GRPC_SERVICE_ENV);
168169
return Boolean.parseBoolean(envVar);
169170
}
170171

@@ -399,12 +400,13 @@ public Image composite(
399400
@Override
400401
public int[][] histogram(Image image) {
401402
if (useGrpc()) {
402-
ImagesHistogramRequest.Builder request =
403+
ImagesHistogramRequest request =
403404
ImagesHistogramRequest.newBuilder()
404-
.setImage(loadImageData(image, BlobstoreServiceFactory.getBlobstoreService()));
405+
.setImage(loadImageData(image, BlobstoreServiceFactory.getBlobstoreService()))
406+
.build();
405407
ImagesHistogramResponse response;
406408
try {
407-
response = getGrpcStub().histogram(request.build());
409+
response = getGrpcStub().histogram(request);
408410
} catch (StatusRuntimeException e) {
409411
throw convertGrpcException(e);
410412
}
@@ -576,7 +578,9 @@ public void deleteServingUrl(BlobKey blobKey) {
576578
try {
577579
byte[] responseBytes = ApiProxy.makeSyncCall(PACKAGE, "DeleteUrlBase",
578580
request.build().toByteArray());
579-
ImagesDeleteUrlBaseResponse unused = ImagesDeleteUrlBaseResponse.parseFrom(responseBytes, ExtensionRegistry.getEmptyRegistry());
581+
ImagesDeleteUrlBaseResponse unused =
582+
ImagesDeleteUrlBaseResponse.parseFrom(
583+
responseBytes, ExtensionRegistry.getEmptyRegistry());
580584
} catch (InvalidProtocolBufferException ex) {
581585
throw new ImagesServiceFailureException("Invalid protocol buffer:", ex);
582586
} catch (ApiProxy.ApplicationException ex) {

‎api/src/test/java/com/google/appengine/api/images/GrpcImagesClientTest.java‎

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ public class GrpcImagesClientTest {
3737

3838
@Test
3939
public void constructor_validEndpointAndCreds_success() {
40-
when(mockEnvironmentProvider.getenv("IMAGES_SERVICE_ENDPOINT"))
40+
when(mockEnvironmentProvider.getenv(ImagesServiceFactoryImpl.IMAGES_SERVICE_ENDPOINT_ENV))
4141
.thenReturn("https://my-service.run.app");
4242

4343
GrpcImagesClient client = new GrpcImagesClient(mockEnvironmentProvider, mockCallCredentials);
@@ -46,37 +46,47 @@ public void constructor_validEndpointAndCreds_success() {
4646

4747
@Test
4848
public void constructor_endpointNotSet_throwsException() {
49-
when(mockEnvironmentProvider.getenv("IMAGES_SERVICE_ENDPOINT")).thenReturn(null);
49+
when(mockEnvironmentProvider.getenv(ImagesServiceFactoryImpl.IMAGES_SERVICE_ENDPOINT_ENV))
50+
.thenReturn(null);
5051
IllegalStateException e =
5152
assertThrows(
5253
IllegalStateException.class,
5354
() -> new GrpcImagesClient(mockEnvironmentProvider, mockCallCredentials));
54-
assertThat(e).hasMessageThat().contains("IMAGES_SERVICE_ENDPOINT environment variable not set");
55+
assertThat(e)
56+
.hasMessageThat()
57+
.contains(
58+
ImagesServiceFactoryImpl.IMAGES_SERVICE_ENDPOINT_ENV + " environment variable not set");
5559
}
5660

5761
@Test
5862
public void constructor_invalidEndpoint_throwsException() {
59-
when(mockEnvironmentProvider.getenv("IMAGES_SERVICE_ENDPOINT")).thenReturn("://my-service");
63+
when(mockEnvironmentProvider.getenv(ImagesServiceFactoryImpl.IMAGES_SERVICE_ENDPOINT_ENV))
64+
.thenReturn("://my-service");
6065
IllegalStateException e =
6166
assertThrows(
6267
IllegalStateException.class,
6368
() -> new GrpcImagesClient(mockEnvironmentProvider, mockCallCredentials));
64-
assertThat(e).hasMessageThat().contains("Invalid URI in IMAGES_SERVICE_ENDPOINT");
69+
assertThat(e)
70+
.hasMessageThat()
71+
.contains("Invalid URI in " + ImagesServiceFactoryImpl.IMAGES_SERVICE_ENDPOINT_ENV);
6572
}
6673

6774
@Test
6875
public void constructor_endpointMissingHost_throwsException() {
69-
when(mockEnvironmentProvider.getenv("IMAGES_SERVICE_ENDPOINT")).thenReturn("https://");
76+
when(mockEnvironmentProvider.getenv(ImagesServiceFactoryImpl.IMAGES_SERVICE_ENDPOINT_ENV))
77+
.thenReturn("https://");
7078
IllegalStateException e =
7179
assertThrows(
7280
IllegalStateException.class,
7381
() -> new GrpcImagesClient(mockEnvironmentProvider, mockCallCredentials));
74-
assertThat(e).hasMessageThat().contains("Invalid URI in IMAGES_SERVICE_ENDPOINT");
82+
assertThat(e)
83+
.hasMessageThat()
84+
.contains("Invalid URI in " + ImagesServiceFactoryImpl.IMAGES_SERVICE_ENDPOINT_ENV);
7585
}
7686

7787
@Test
7888
public void getBlockingStub_returnsStub() {
79-
when(mockEnvironmentProvider.getenv("IMAGES_SERVICE_ENDPOINT"))
89+
when(mockEnvironmentProvider.getenv(ImagesServiceFactoryImpl.IMAGES_SERVICE_ENDPOINT_ENV))
8090
.thenReturn("https://my-service.run.app");
8191
GrpcImagesClient client = new GrpcImagesClient(mockEnvironmentProvider, mockCallCredentials);
8292
assertThat(client.getBlockingStub()).isNotNull();

‎api/src/test/java/com/google/appengine/api/images/ImagesServiceFactoryImplTest.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,9 @@ public void setUp() {
4141

4242
@Test
4343
public void makeImageFromFilename_newBehavior_trueEnv_gsPrefix() {
44-
when(mockEnvironmentProvider.getenv("USE_CUSTOM_IMAGES_GRPC_SERVICE")).thenReturn("true");
44+
when(mockEnvironmentProvider.getenv(
45+
ImagesServiceFactoryImpl.USE_CUSTOM_IMAGES_GRPC_SERVICE_ENV))
46+
.thenReturn("true");
4547
String filename = "/gs/bucket/object";
4648

4749
// Should NOT call BlobstoreServiceFactory (which would fail in this env)
@@ -53,7 +55,9 @@ public void makeImageFromFilename_newBehavior_trueEnv_gsPrefix() {
5355

5456
@Test
5557
public void makeImageFromFilename_newBehavior_trueEnv_noGsPrefix_throwsException() {
56-
when(mockEnvironmentProvider.getenv("USE_CUSTOM_IMAGES_GRPC_SERVICE")).thenReturn("true");
58+
when(mockEnvironmentProvider.getenv(
59+
ImagesServiceFactoryImpl.USE_CUSTOM_IMAGES_GRPC_SERVICE_ENV))
60+
.thenReturn("true");
5761
String filename = "not/gs/path";
5862

5963
IllegalArgumentException e =

‎api/src/test/java/com/google/appengine/api/images/ImagesServiceImplTest.java‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,9 @@ private void setupGrpcService(ImagesServiceGrpc.ImagesServiceImplBase serviceImp
172172
new ImagesServiceImpl(
173173
mockEnvironmentProvider, mockGrpcImagesClient, null, mockBlobstoreReference);
174174

175-
when(mockEnvironmentProvider.getenv("USE_CUSTOM_IMAGES_GRPC_SERVICE")).thenReturn("true");
175+
when(mockEnvironmentProvider.getenv(
176+
ImagesServiceFactoryImpl.USE_CUSTOM_IMAGES_GRPC_SERVICE_ENV))
177+
.thenReturn("true");
176178
}
177179

178180
@Test
@@ -354,7 +356,8 @@ public void loadImageData_withLegacyBlobKey_failure() throws Exception {
354356

355357
public void setUpGrpc(boolean useGrpc) throws Exception {
356358
ImagesServiceImpl.setStorageForTesting(mockStorage);
357-
when(mockEnvironmentProvider.getenv("USE_CUSTOM_IMAGES_GRPC_SERVICE"))
359+
when(mockEnvironmentProvider.getenv(
360+
ImagesServiceFactoryImpl.USE_CUSTOM_IMAGES_GRPC_SERVICE_ENV))
358361
.thenReturn(Boolean.toString(useGrpc));
359362

360363
if (useGrpc) {
@@ -407,7 +410,9 @@ public void useGrpc_envVarSetFalse_returnsFalse() throws Exception {
407410

408411
@Test
409412
public void useGrpc_envVarNotSet_returnsFalse() {
410-
when(mockEnvironmentProvider.getenv("USE_CUSTOM_IMAGES_GRPC_SERVICE")).thenReturn(null);
413+
when(mockEnvironmentProvider.getenv(
414+
ImagesServiceFactoryImpl.USE_CUSTOM_IMAGES_GRPC_SERVICE_ENV))
415+
.thenReturn(null);
411416
imagesService =
412417
new ImagesServiceImpl(
413418
mockEnvironmentProvider, null, null, mockBlobstoreReference, null, mockBlobInfoFactory);
@@ -416,7 +421,9 @@ public void useGrpc_envVarNotSet_returnsFalse() {
416421

417422
@Test
418423
public void useGrpc_envVarInvalid_returnsFalse() {
419-
when(mockEnvironmentProvider.getenv("USE_CUSTOM_IMAGES_GRPC_SERVICE")).thenReturn("yes");
424+
when(mockEnvironmentProvider.getenv(
425+
ImagesServiceFactoryImpl.USE_CUSTOM_IMAGES_GRPC_SERVICE_ENV))
426+
.thenReturn("yes");
420427
imagesService =
421428
new ImagesServiceImpl(
422429
mockEnvironmentProvider, null, null, mockBlobstoreReference, null, mockBlobInfoFactory);

0 commit comments

Comments
 (0)