Merge branch 'main' into da_AJ-1011_noInstanceInGetJob

service/src/main/java/org/databiosphere/workspacedataservice/activitylog/ActivityEventBuilder.java

@@ -3,6 +3,7 @@
 import java.util.UUID;
 import org.databiosphere.workspacedataservice.sam.SamDao;
 import org.databiosphere.workspacedataservice.sam.TokenContextUtil;
+import org.databiosphere.workspacedataservice.shared.model.BearerToken;
 import org.databiosphere.workspacedataservice.shared.model.RecordType;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -36,10 +37,10 @@ public ActivityEventBuilder(SamDao samDao) {
   public ActivityEventBuilder currentUser() {
     try {
       // grab the current user's bearer token (see BearerTokenFilter)
-      String token = TokenContextUtil.getToken();
-      if (token != null) {
+      BearerToken token = TokenContextUtil.getToken();
+      if (token.nonEmpty()) {
         // resolve the token to a user id via Sam
-        this.subject = samDao.getUserId(token);
+        this.subject = samDao.getUserId(token.getValue());
       } else {
         this.subject = "anonymous";
       }

service/src/main/java/org/databiosphere/workspacedataservice/datarepo/HttpDataRepoClientFactory.java

@@ -5,6 +5,7 @@
 import javax.ws.rs.client.Client;
 import org.apache.commons.lang3.StringUtils;
 import org.databiosphere.workspacedataservice.sam.TokenContextUtil;
+import org.databiosphere.workspacedataservice.shared.model.BearerToken;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -29,11 +30,11 @@ private ApiClient getApiClient() {
       apiClient.setBasePath(dataRepoUrl);
     }
     // grab the current user's bearer token (see BearerTokenFilter)
-    String token = TokenContextUtil.getToken();
+    BearerToken token = TokenContextUtil.getToken();
     // add the user's bearer token to the client
-    if (token != null) {
+    if (token.nonEmpty()) {
       LOGGER.debug("setting access token for data repo request");
-      apiClient.setAccessToken(token);
+      apiClient.setAccessToken(token.getValue());
     } else {
       LOGGER.warn("No access token found for data repo request.");
     }

service/src/main/java/org/databiosphere/workspacedataservice/leonardo/HttpLeonardoClientFactory.java

@@ -7,6 +7,7 @@
 import org.broadinstitute.dsde.workbench.client.leonardo.ApiClient;
 import org.broadinstitute.dsde.workbench.client.leonardo.api.AppsApi;
 import org.databiosphere.workspacedataservice.sam.TokenContextUtil;
+import org.databiosphere.workspacedataservice.shared.model.BearerToken;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -35,11 +36,11 @@ private ApiClient getApiClient(String authToken) {
     }
 
     // grab the current user's bearer token (see BearerTokenFilter) or use parameter value
-    String token = TokenContextUtil.getToken(authToken);
+    BearerToken token = TokenContextUtil.getToken(authToken);
     // add the user's bearer token to the client
-    if (token != null) {
+    if (token.nonEmpty()) {
       LOGGER.debug("setting access token for leonardo request");
-      apiClient.setAccessToken(token);
+      apiClient.setAccessToken(token.getValue());
     } else {
       LOGGER.warn("No access token found for leonardo request.");
     }

service/src/main/java/org/databiosphere/workspacedataservice/sam/HttpSamClientFactory.java

@@ -1,14 +1,14 @@
 package org.databiosphere.workspacedataservice.sam;
 
 import java.util.List;
-import java.util.Objects;
 import okhttp3.OkHttpClient;
 import okhttp3.Protocol;
 import org.apache.commons.lang3.StringUtils;
 import org.broadinstitute.dsde.workbench.client.sam.ApiClient;
 import org.broadinstitute.dsde.workbench.client.sam.api.ResourcesApi;
 import org.broadinstitute.dsde.workbench.client.sam.api.StatusApi;
 import org.broadinstitute.dsde.workbench.client.sam.api.UsersApi;
+import org.databiosphere.workspacedataservice.shared.model.BearerToken;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -44,12 +44,12 @@ private ApiClient getApiClient(String authToken) {
     }
 
     // grab the current user's bearer token (see BearerTokenFilter) or use parameter value
-    String token = TokenContextUtil.getToken(authToken);
+    BearerToken token = TokenContextUtil.getToken(authToken);
 
     // add the user's bearer token to the client
-    if (!Objects.isNull(token)) {
+    if (token.nonEmpty()) {
       LOGGER.debug("setting access token for Sam request");
-      apiClient.setAccessToken(token);
+      apiClient.setAccessToken(token.getValue());
     } else {
       LOGGER.warn("No access token found for Sam request.");
     }

service/src/main/java/org/databiosphere/workspacedataservice/sam/TokenContextUtil.java

@@ -5,7 +5,9 @@
 
 import java.util.Map;
 import java.util.function.Supplier;
+import javax.validation.constraints.NotNull;
 import org.databiosphere.workspacedataservice.jobexec.JobContextHolder;
+import org.databiosphere.workspacedataservice.shared.model.BearerToken;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.web.context.request.RequestAttributes;
@@ -33,8 +35,9 @@ private TokenContextUtil() {}
    * @param initialValue the first value to check for a non-null token
    * @return the final value
    */
-  public static String getToken(String initialValue) {
-    return getToken(initialValue, () -> null);
+  @NotNull
+  public static BearerToken getToken(String initialValue) {
+    return getToken(BearerToken.of(initialValue), BearerToken::empty);
   }
 
   /**
@@ -54,8 +57,9 @@ public static String getToken(String initialValue) {
    * @param orElse the value to return if the token was not found otherwise
    * @return the final value
    */
-  public static String getToken(String initialValue, Supplier<String> orElse) {
-    if (initialValue != null) {
+  @NotNull
+  public static BearerToken getToken(BearerToken initialValue, Supplier<BearerToken> orElse) {
+    if (initialValue != null && initialValue.nonEmpty()) {
       return initialValue;
     }
     return getToken(orElse);
@@ -68,9 +72,10 @@ public static String getToken(String initialValue, Supplier<String> orElse) {
    * @param orElse the value to return if the token was not found otherwise
    * @return the final value
    */
-  public static String getToken(Supplier<String> orElse) {
-    String foundToken = getToken();
-    if (foundToken != null) {
+  @NotNull
+  public static BearerToken getToken(Supplier<BearerToken> orElse) {
+    BearerToken foundToken = getToken();
+    if (foundToken.nonEmpty()) {
       // N.B. no logging here; this is the simplest case
       return foundToken;
     } else {
@@ -83,20 +88,21 @@ public static String getToken(Supplier<String> orElse) {
    * Look in RequestContextHolder and then JobContextHolder, in that order, for a non-null String
    * ATTRIBUTE_NAME_TOKEN
    *
-   * @return the token if found; null otherwise
+   * @return the token if found; BearerToken.empty() otherwise
    */
-  public static String getToken() {
-    String foundToken;
+  @NotNull
+  public static BearerToken getToken() {
+    BearerToken foundToken;
     // look in request context; if non-null, return it
     foundToken = tokenFromRequestContext();
-    if (foundToken != null) {
+    if (foundToken.nonEmpty()) {
       LOGGER.debug("Token retrieved from request context.");
       return foundToken;
     }
 
     // look in job context; return whatever we found, even if null
     foundToken = tokenFromJobContext();
-    if (foundToken != null) {
+    if (foundToken.nonEmpty()) {
       LOGGER.debug("Token retrieved from job context.");
     }
     return foundToken;
@@ -105,36 +111,42 @@ public static String getToken() {
   /**
    * Look in RequestContextHolder for a non-null String ATTRIBUTE_NAME_TOKEN
    *
-   * @return the token if found; null otherwise
+   * @return the token if found; BearerToken.empty() otherwise
    */
-  private static String tokenFromRequestContext() {
+  @NotNull
+  private static BearerToken tokenFromRequestContext() {
     // do any request attributes exist?
     RequestAttributes requestAttributes = RequestContextHolder.getRequestAttributes();
     if (requestAttributes != null) {
       return maybeToken(requestAttributes.getAttribute(ATTRIBUTE_NAME_TOKEN, SCOPE_REQUEST));
     }
-    return null;
+    return BearerToken.empty();
   }
 
   /**
    * Look in JobContextHolder for a non-null String ATTRIBUTE_NAME_TOKEN
    *
-   * @return the token if found; null otherwise
+   * @return the token if found; BearerToken.empty() otherwise
    */
-  private static String tokenFromJobContext() {
+  @NotNull
+  private static BearerToken tokenFromJobContext() {
     // do any job attributes exist?
     Map<String, Object> jobAttributes = JobContextHolder.getAttributes();
-    if (jobAttributes == null) {
-      return null;
+    if (jobAttributes != null) {
+      return maybeToken(jobAttributes.get(ATTRIBUTE_NAME_TOKEN));
     }
-    return maybeToken(jobAttributes.get(ATTRIBUTE_NAME_TOKEN));
+    return BearerToken.empty();
   }
 
   /** Convenience: is the input object non-null and a String? */
-  private static String maybeToken(Object obj) {
+  @NotNull
+  private static BearerToken maybeToken(Object obj) {
+    // as of this writing, if "obj instanceof String" passes, then "BearerToken.isValid" will always
+    // pass. The check is included here for future compatibility, in case we change the isValid
+    // implementation later.
     if (obj instanceof String strVal) {
-      return strVal;
+      return BearerToken.of(strVal);
     }
-    return null;
+    return BearerToken.empty();
   }
 }

service/src/main/java/org/databiosphere/workspacedataservice/service/ImportService.java

@@ -18,6 +18,7 @@
 import org.databiosphere.workspacedataservice.sam.SamDao;
 import org.databiosphere.workspacedataservice.sam.TokenContextUtil;
 import org.databiosphere.workspacedataservice.service.model.exception.AuthorizationException;
+import org.databiosphere.workspacedataservice.shared.model.BearerToken;
 import org.databiosphere.workspacedataservice.shared.model.Schedulable;
 import org.databiosphere.workspacedataservice.shared.model.job.Job;
 import org.databiosphere.workspacedataservice.shared.model.job.JobInput;
@@ -69,12 +70,12 @@ public GenericJobServerModel createImport(
 
     // get the user's token from the current request
     // TODO: this should actually get a pet token for the user, to ensure the token won't time out
-    String token = TokenContextUtil.getToken();
+    BearerToken token = TokenContextUtil.getToken();
 
     // create the arguments for the schedulable job
     Map<String, Serializable> arguments = new HashMap<>();
-    if (token != null) {
-      arguments.put(ARG_TOKEN, token);
+    if (token.nonEmpty()) {
+      arguments.put(ARG_TOKEN, token.getValue());
     }
     arguments.put(ARG_URL, importRequest.getUrl().toString());
     arguments.put(ARG_INSTANCE, instanceUuid.toString());

service/src/main/java/org/databiosphere/workspacedataservice/shared/model/BearerToken.java

@@ -0,0 +1,52 @@
+package org.databiosphere.workspacedataservice.shared.model;
+
+import java.util.Objects;
+
+/**
+ * Record class to represent an auth token; use this instead of String for more type safety and
+ * readability of argument lists. As of this writing, WDS includes both Nimbus and Apache Oltu as
+ * transitive dependencies, and both of those libraries include models to represent a token; but
+ * they also both have more complexity than we need, and we don't want to depend too much on
+ * transitive dependencies
+ */
+public class BearerToken {
+
+  private final String value;
+
+  private BearerToken(String value) {
+    this.value = value;
+  }
+
+  /** returns an empty BearerToken; that is, a BearerToken whose `value` is null */
+  public static BearerToken empty() {
+    return new BearerToken(null);
+  }
+
+  /** returns a BearerToken with the specified `value` */
+  public static BearerToken of(String val) {
+    return new BearerToken(val);
+  }
+
+  public String getValue() {
+    return value;
+  }
+
+  /** indicates if this BearerToken is empty; that is, has a null `value` */
+  public boolean isEmpty() {
+    return value == null;
+  }
+
+  /** indicates if this BearerToken is not empty; that is, has a non-null `value` */
+  public boolean nonEmpty() {
+    return !isEmpty();
+  }
+
+  // generated by IntelliJ
+  @Override
+  public boolean equals(Object o) {
+    if (this == o) return true;
+    if (o == null || getClass() != o.getClass()) return false;
+    BearerToken that = (BearerToken) o;
+    return Objects.equals(value, that.value);
+  }
+}

service/src/main/java/org/databiosphere/workspacedataservice/sourcewds/HttpWorkspaceDataServiceClientFactory.java

@@ -7,6 +7,7 @@
 import org.databiosphere.workspacedata.api.CloningApi;
 import org.databiosphere.workspacedata.client.ApiClient;
 import org.databiosphere.workspacedataservice.sam.TokenContextUtil;
+import org.databiosphere.workspacedataservice.shared.model.BearerToken;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -34,12 +35,12 @@ private ApiClient getApiClient(String authToken, String workspaceDataServiceUrl)
     }
 
     // grab the current user's bearer token (see BearerTokenFilter) or use parameter value
-    String token = TokenContextUtil.getToken(authToken);
+    BearerToken token = TokenContextUtil.getToken(authToken);
 
     // add the user's bearer token to the client
-    if (token != null) {
+    if (token.nonEmpty()) {
       LOGGER.debug("setting access token for workspace data service request");
-      apiClient.setAccessToken(token);
+      apiClient.setAccessToken(token.getValue());
     } else {
       LOGGER.warn("No access token found for workspace data service request.");
     }

service/src/main/java/org/databiosphere/workspacedataservice/workspacemanager/HttpWorkspaceManagerClientFactory.java

@@ -7,6 +7,7 @@
 import javax.ws.rs.client.Client;
 import org.apache.commons.lang3.StringUtils;
 import org.databiosphere.workspacedataservice.sam.TokenContextUtil;
+import org.databiosphere.workspacedataservice.shared.model.BearerToken;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -33,12 +34,12 @@ private ApiClient getApiClient(String authToken) {
     }
 
     // grab the current user's bearer token (see BearerTokenFilter) or use parameter value
-    String token = TokenContextUtil.getToken(authToken);
+    BearerToken token = TokenContextUtil.getToken(authToken);
 
     // add the user's bearer token to the client
-    if (token != null) {
+    if (token.nonEmpty()) {
       LOGGER.debug("setting access token for workspace manager request");
-      apiClient.setAccessToken(token);
+      apiClient.setAccessToken(token.getValue());
     } else {
       LOGGER.warn("No access token found for workspace manager request.");
     }

service/src/test/java/org/databiosphere/workspacedataservice/jobexec/QuartzJobTest.java

@@ -2,7 +2,7 @@
 
 import static org.databiosphere.workspacedataservice.shared.model.Schedulable.ARG_TOKEN;
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -71,7 +71,7 @@ protected JobDao getJobDao() {
 
     @Override
     protected void executeInternal(UUID jobId, JobExecutionContext context) {
-      assertEquals(expectedToken, TokenContextUtil.getToken());
+      assertEquals(expectedToken, TokenContextUtil.getToken().getValue());
     }
   }
 
@@ -87,11 +87,13 @@ void tokenIsStashedAndCleaned() throws JobExecutionException {
     when(mockContext.getJobDetail()).thenReturn(jobDetail);
 
     // assert that no token exists via TokenContextUtil
-    assertNull(TokenContextUtil.getToken(), "no token should exist before running the job");
+    assertTrue(
+        TokenContextUtil.getToken().isEmpty(), "no token should exist before running the job");
     // execute the TestableQuartzJob, which asserts that the token passed properly from the
     // JobDataMap into job context and is therefore retrievable via TokenContextUtil
     new TestableQuartzJob(expectedToken).execute(mockContext);
     // assert that the token is cleaned up and no longer reachable via TokenContextUtil
-    assertNull(TokenContextUtil.getToken(), "no token should exist after running the job");
+    assertTrue(
+        TokenContextUtil.getToken().isEmpty(), "no token should exist after running the job");
   }
 }