From 35e85cee99e43135974e165de702cd16d632ef18 Mon Sep 17 00:00:00 2001
From: Max Lambrecht <maxlambrecht@gmail.com>
Date: Thu, 2 Jul 2020 11:39:02 -0300
Subject: [PATCH] Minor refactors and documentation clarifying the watch
 methods in WorkloadApiClient.

Signed-off-by: Max Lambrecht <maxlambrecht@gmail.com>
---
 .../workloadapi/DefaultWorkloadApiClient.java | 26 +++++++++++++------
 .../spiffe/workloadapi/WorkloadApiClient.java |  8 ++++++
 .../DefaultWorkloadApiClientTest.java         |  3 +--
 3 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/java-spiffe-core/src/main/java/io/spiffe/workloadapi/DefaultWorkloadApiClient.java b/java-spiffe-core/src/main/java/io/spiffe/workloadapi/DefaultWorkloadApiClient.java
index c89ea22..a0e32d9 100644
--- a/java-spiffe-core/src/main/java/io/spiffe/workloadapi/DefaultWorkloadApiClient.java
+++ b/java-spiffe-core/src/main/java/io/spiffe/workloadapi/DefaultWorkloadApiClient.java
@@ -190,9 +190,7 @@ public final class DefaultWorkloadApiClient implements WorkloadApiClient {
                                 final String... extraAudience)
             throws JwtSvidException {
 
-        final Set<String> audParam = new HashSet<>();
-        audParam.add(audience);
-        Collections.addAll(audParam, extraAudience);
+        final Set<String> audParam = createAudienceSet(audience, extraAudience);
 
         try (val cancellableContext = Context.current().withCancellation()) {
             return cancellableContext.call(() -> callFetchJwtSvid(subject, audParam));
@@ -219,11 +217,8 @@ public final class DefaultWorkloadApiClient implements WorkloadApiClient {
     @Override
     public JwtSvid validateJwtSvid(@NonNull final String token, @NonNull final String audience)
             throws JwtSvidException {
-        val request = Workload.ValidateJWTSVIDRequest
-                .newBuilder()
-                .setSvid(token)
-                .setAudience(audience)
-                .build();
+
+        val request = createJwtSvidRequest(token, audience);
 
         try (val cancellableContext = Context.current().withCancellation()) {
             cancellableContext.call(() -> workloadApiBlockingStub.validateJWTSVID(request));
@@ -308,6 +303,13 @@ public final class DefaultWorkloadApiClient implements WorkloadApiClient {
         throw new JwtBundleException("JWT Bundle response from the Workload API is empty");
     }
 
+    private Set<String> createAudienceSet(final @NonNull String audience, final String[] extraAudience) {
+        final Set<String> audParam = new HashSet<>();
+        audParam.add(audience);
+        Collections.addAll(audParam, extraAudience);
+        return audParam;
+    }
+
     private Workload.X509SVIDRequest newX509SvidRequest() {
         return Workload.X509SVIDRequest.newBuilder().build();
     }
@@ -316,6 +318,14 @@ public final class DefaultWorkloadApiClient implements WorkloadApiClient {
         return Workload.JWTBundlesRequest.newBuilder().build();
     }
 
+    private Workload.ValidateJWTSVIDRequest createJwtSvidRequest(final @NonNull String token, final @NonNull String audience) {
+        return Workload.ValidateJWTSVIDRequest
+                .newBuilder()
+                .setSvid(token)
+                .setAudience(audience)
+                .build();
+    }
+
     /**
      * Options for creating a new {@link DefaultWorkloadApiClient}.
      * <p>
diff --git a/java-spiffe-core/src/main/java/io/spiffe/workloadapi/WorkloadApiClient.java b/java-spiffe-core/src/main/java/io/spiffe/workloadapi/WorkloadApiClient.java
index e56c7e3..658f8df 100644
--- a/java-spiffe-core/src/main/java/io/spiffe/workloadapi/WorkloadApiClient.java
+++ b/java-spiffe-core/src/main/java/io/spiffe/workloadapi/WorkloadApiClient.java
@@ -27,6 +27,10 @@ public interface WorkloadApiClient extends Closeable {
 
     /**
      * Watches for X.509 context updates.
+     * <p>
+     * A new Stream to the Workload API is opened for each call to this method, so that the client start getting
+     * updates immediately after the Stream is ready and doesn't have to wait until the Workload API dispatches
+     * the next update based on the SVIDs TTL.
      *
      * @param watcher an instance that implements a {@link Watcher}.
      */
@@ -63,6 +67,10 @@ public interface WorkloadApiClient extends Closeable {
 
     /**
      * Watches for JWT bundles updates.
+     * <p>
+     * A new Stream to the Workload API is opened for each call to this method, so that the client start getting
+     * updates immediately after the Stream is ready and doesn't have to wait until the Workload API dispatches
+     * the next update based on the SVIDs TTL.
      *
      * @param watcher receives the update for JwtBundles.
      */
diff --git a/java-spiffe-core/src/test/java/io/spiffe/workloadapi/DefaultWorkloadApiClientTest.java b/java-spiffe-core/src/test/java/io/spiffe/workloadapi/DefaultWorkloadApiClientTest.java
index 8283a49..3adb895 100644
--- a/java-spiffe-core/src/test/java/io/spiffe/workloadapi/DefaultWorkloadApiClientTest.java
+++ b/java-spiffe-core/src/test/java/io/spiffe/workloadapi/DefaultWorkloadApiClientTest.java
@@ -46,7 +46,6 @@ class DefaultWorkloadApiClientTest {
     @Rule
     public final GrpcCleanupRule grpcCleanup = new GrpcCleanupRule();
     private DefaultWorkloadApiClient workloadApiClient;
-    private ManagedChannel inProcessChannel;
 
     @BeforeEach
     void setUp() throws IOException {
@@ -59,7 +58,7 @@ class DefaultWorkloadApiClientTest {
         grpcCleanup.register(server);
 
         // Create WorkloadApiClient using Stubs that will connect to the fake WorkloadApiService.
-        inProcessChannel = InProcessChannelBuilder.forName(serverName).directExecutor().build();
+        final ManagedChannel inProcessChannel = InProcessChannelBuilder.forName(serverName).directExecutor().build();
         grpcCleanup.register(inProcessChannel);
 
         SpiffeWorkloadAPIGrpc.SpiffeWorkloadAPIBlockingStub workloadApiBlockingStub = SpiffeWorkloadAPIGrpc