Skip to content

Commit 6cf7698

Browse files
committed
Address comments (add javadoc and use assertThrows in tests)
1 parent 97e7147 commit 6cf7698

File tree

2 files changed

+39
-31
lines changed

2 files changed

+39
-31
lines changed

cab-token-generator/javatests/com/google/auth/credentialaccessboundary/ClientSideCredentialAccessBoundaryFactoryTest.java

Lines changed: 31 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@
3535
import static org.junit.Assert.assertEquals;
3636
import static org.junit.Assert.assertNotNull;
3737
import static org.junit.Assert.assertThrows;
38-
import static org.junit.Assert.fail;
3938
import static org.mockito.Mockito.mock;
4039
import static org.mockito.Mockito.when;
4140

@@ -136,8 +135,7 @@ public void fetchIntermediateCredentials() throws Exception {
136135
public void fetchIntermediateCredentials_withCustomUniverseDomain() throws IOException {
137136
String universeDomain = "foobar";
138137
GoogleCredentials sourceCredentials =
139-
getServiceAccountSourceCredentials(mockTokenServerTransportFactory)
140-
.toBuilder()
138+
getServiceAccountSourceCredentials(mockTokenServerTransportFactory).toBuilder()
141139
.setUniverseDomain(universeDomain)
142140
.build();
143141

@@ -360,25 +358,21 @@ public void refreshCredentialsIfRequired_sourceCredentialCannotRefresh_throwsIOE
360358
.setHttpTransportFactory(mockStsTransportFactory)
361359
.build();
362360

363-
try {
364-
factory.refreshCredentialsIfRequired(); // Expecting an IOException
365-
fail("Should fail as the source credential should not be able to be refreshed.");
366-
} catch (IOException e) {
367-
assertEquals("Unable to refresh the provided source credential.", e.getMessage());
368-
}
361+
IOException exception = assertThrows(IOException.class, factory::refreshCredentialsIfRequired);
362+
assertEquals("Unable to refresh the provided source credential.", exception.getMessage());
369363
}
370364

371365
// Tests related to the builder methods
372366
@Test
373367
public void builder_noSourceCredential_throws() {
374-
try {
375-
ClientSideCredentialAccessBoundaryFactory.newBuilder()
376-
.setHttpTransportFactory(OAuth2Utils.HTTP_TRANSPORT_FACTORY)
377-
.build();
378-
fail("Should fail as the source credential is null.");
379-
} catch (NullPointerException e) {
380-
assertEquals("Source credential must not be null.", e.getMessage());
381-
}
368+
NullPointerException exception =
369+
assertThrows(
370+
NullPointerException.class,
371+
() ->
372+
ClientSideCredentialAccessBoundaryFactory.newBuilder()
373+
.setHttpTransportFactory(OAuth2Utils.HTTP_TRANSPORT_FACTORY)
374+
.build());
375+
assertEquals("Source credential must not be null.", exception.getMessage());
382376
}
383377

384378
@Test
@@ -461,18 +455,17 @@ public void builder_universeDomainMismatch_throws() throws IOException {
461455
GoogleCredentials sourceCredentials =
462456
getServiceAccountSourceCredentials(mockTokenServerTransportFactory);
463457

464-
try {
465-
ClientSideCredentialAccessBoundaryFactory.newBuilder()
466-
.setSourceCredential(sourceCredentials)
467-
.setUniverseDomain("differentUniverseDomain")
468-
.build();
469-
470-
fail("Should fail with universe domain mismatch.");
471-
} catch (IllegalArgumentException e) {
472-
assertEquals(
473-
"The client side access boundary credential's universe domain must be the same as the source credential.",
474-
e.getMessage());
475-
}
458+
IllegalArgumentException exception =
459+
assertThrows(
460+
IllegalArgumentException.class,
461+
() ->
462+
ClientSideCredentialAccessBoundaryFactory.newBuilder()
463+
.setSourceCredential(sourceCredentials)
464+
.setUniverseDomain("differentUniverseDomain")
465+
.build());
466+
assertEquals(
467+
"The client side access boundary credential's universe domain must be the same as the source credential.",
468+
exception.getMessage());
476469
}
477470

478471
private static GoogleCredentials getServiceAccountSourceCredentials(
@@ -548,6 +541,7 @@ private static void triggerConcurrentRefresh(
548541
throws InterruptedException {
549542
Thread[] threads = new Thread[numThreads];
550543
CountDownLatch latch = new CountDownLatch(numThreads);
544+
long timeoutMillis = 5000; // 5 seconds
551545

552546
// Create and start the threads
553547
for (int i = 0; i < numThreads; i++) {
@@ -567,7 +561,14 @@ private static void triggerConcurrentRefresh(
567561
threads[i].start();
568562
}
569563
for (Thread thread : threads) {
570-
thread.join(); // Wait for each thread to complete
564+
thread.join(timeoutMillis); // Wait for each thread to complete
565+
if (thread.isAlive()) {
566+
thread.interrupt(); // Interrupt the thread if it's still running after the timeout
567+
throw new AssertionError(
568+
"Thread running refreshCredentialsIfRequired timed out after "
569+
+ timeoutMillis
570+
+ " milliseconds.");
571+
}
571572
}
572573
}
573574
}

oauth2_http/java/com/google/auth/oauth2/OAuth2Utils.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,14 @@ static Map<String, Object> validateMap(Map<String, Object> map, String key, Stri
256256
return (Map) value;
257257
}
258258

259-
/** Helper to convert from a PKCS#8 String to an RSA private key */
259+
/**
260+
* Converts a PKCS#8 string to an RSA private key.
261+
*
262+
* @param privateKeyPkcs8 the PKCS#8 string.
263+
* @return the RSA private key.
264+
* @throws IOException if the PKCS#8 data is invalid or if an unexpected exception occurs during
265+
* key creation.
266+
*/
260267
public static PrivateKey privateKeyFromPkcs8(String privateKeyPkcs8) throws IOException {
261268
Reader reader = new StringReader(privateKeyPkcs8);
262269
Section section = PemReader.readFirstSectionAndClose(reader, "PRIVATE KEY");

0 commit comments

Comments
 (0)