Merge pull request #653 from DwayneJengSage/develop

BRIDGE-3390 ClientInfo Fixes
Sage-Bionetworks · Apr 6, 2023 · ad5db4b · ad5db4b
2 parents ab35fcf + 6061af5
commit ad5db4b
Show file tree

Hide file tree

Showing 23 changed files with 170 additions and 124 deletions.
diff --git a/src/main/java/org/sagebionetworks/bridge/RequestContext.java b/src/main/java/org/sagebionetworks/bridge/RequestContext.java
@@ -1,7 +1,5 @@
 package org.sagebionetworks.bridge;
 
-import static org.sagebionetworks.bridge.models.ClientInfo.UNKNOWN_CLIENT;
-
 import java.util.List;
 import java.util.Set;
 
@@ -23,7 +21,8 @@ public class RequestContext {
     private static final ThreadLocal<RequestContext> REQUEST_CONTEXT_THREAD_LOCAL = ThreadLocal.withInitial(() -> null);
 
     public static final RequestContext NULL_INSTANCE = new RequestContext(null, null, null, null, ImmutableSet.of(),
-            ImmutableSet.of(), ImmutableSet.of(), null, UNKNOWN_CLIENT, ImmutableList.of(), null);
+            ImmutableSet.of(), ImmutableSet.of(), null, ImmutableList.of(), null,
+            null);
 
     /** Gets the request context for the current thread. See also RequestInterceptor. */
     public static RequestContext get() {
@@ -71,21 +70,23 @@ public static RequestContext updateFromSession(UserSession session, SponsorServi
     private final List<String> callerLanguages;    
     private final Metrics metrics;
     private final String callerIpAddress;
+    private final String userAgent;
 
     private RequestContext(Metrics metrics, String requestId, String callerAppId, String callerOrgMembership,
             Set<String> callerEnrolledStudies, Set<String> orgSponsoredStudies, Set<Roles> callerRoles,
-            String callerUserId, ClientInfo callerClientInfo, List<String> callerLanguages, String callerIpAddress) {
+            String callerUserId, List<String> callerLanguages, String callerIpAddress, String userAgent) {
         this.requestId = requestId;
         this.callerAppId = callerAppId;
         this.callerOrgMembership = callerOrgMembership;
         this.callerEnrolledStudies = callerEnrolledStudies;
         this.orgSponsoredStudies = orgSponsoredStudies;
         this.callerRoles = callerRoles;
         this.callerUserId = callerUserId;
-        this.callerClientInfo = callerClientInfo;
+        this.callerClientInfo = ClientInfo.fromUserAgentCache(userAgent);
         this.callerLanguages = callerLanguages;
         this.metrics = metrics;
         this.callerIpAddress = callerIpAddress;
+        this.userAgent = userAgent;
     }
 
     public Metrics getMetrics() {
@@ -132,19 +133,25 @@ public List<String> getCallerLanguages() {
     public String getCallerIpAddress() {
         return callerIpAddress;
     }
+
+    /** The user's User-Agent header. */
+    public String getUserAgent() {
+        return userAgent;
+    }
+
     public RequestContext.Builder toBuilder() {
         return new RequestContext.Builder()
-            .withRequestId(requestId)
-            .withCallerClientInfo(callerClientInfo)
-            .withCallerAppId(callerAppId)
-            .withCallerOrgMembership(callerOrgMembership)
-            .withCallerLanguages(callerLanguages)
-            .withCallerRoles(callerRoles)
-            .withCallerEnrolledStudies(callerEnrolledStudies)
-            .withOrgSponsoredStudies(orgSponsoredStudies)
-            .withCallerUserId(callerUserId)
-            .withMetrics(metrics)
-            .withCallerIpAddress(callerIpAddress);
+                .withRequestId(requestId)
+                .withCallerAppId(callerAppId)
+                .withCallerOrgMembership(callerOrgMembership)
+                .withCallerLanguages(callerLanguages)
+                .withCallerRoles(callerRoles)
+                .withCallerEnrolledStudies(callerEnrolledStudies)
+                .withOrgSponsoredStudies(orgSponsoredStudies)
+                .withCallerUserId(callerUserId)
+                .withMetrics(metrics)
+                .withCallerIpAddress(callerIpAddress)
+                .withUserAgent(userAgent);
     }
 
     public static class Builder {
@@ -156,9 +163,9 @@ public static class Builder {
         private Set<Roles> callerRoles;
         private String requestId;
         private String callerUserId;
-        private ClientInfo callerClientInfo;
         private List<String> callerLanguages;
         private String callerIpAddress;
+        private String userAgent;
 
         public Builder withMetrics(Metrics metrics) {
             this.metrics = metrics;
@@ -194,10 +201,6 @@ public Builder withCallerUserId(String callerUserId) {
             this.callerUserId = callerUserId;
             return this;
         }
-        public Builder withCallerClientInfo(ClientInfo callerClientInfo) {
-            this.callerClientInfo = callerClientInfo;
-            return this;
-        }
         public Builder withCallerLanguages(List<String> callerLanguages) {
             this.callerLanguages = callerLanguages;
             return this;
@@ -206,6 +209,12 @@ public Builder withCallerIpAddress(String callerIpAddress) {
             this.callerIpAddress = callerIpAddress;
             return this;
         }
+
+        /** The user's User-Agent header. */
+        public Builder withUserAgent(String userAgent) {
+            this.userAgent = userAgent;
+            return this;
+        }
 
         public RequestContext build() {
             if (requestId == null) {
@@ -223,14 +232,12 @@ public RequestContext build() {
             if (callerLanguages == null) {
                 callerLanguages = ImmutableList.of();
             }
-            if (callerClientInfo == null) {
-                callerClientInfo = ClientInfo.UNKNOWN_CLIENT;
-            }
             if (metrics == null) {
                 metrics = new Metrics(requestId);
             }
             return new RequestContext(metrics, requestId, callerAppId, callerOrgMembership, callerEnrolledStudies,
-                    orgSponsoredStudies, callerRoles, callerUserId, callerClientInfo, callerLanguages, callerIpAddress);
+                    orgSponsoredStudies, callerRoles, callerUserId, callerLanguages, callerIpAddress,
+                    userAgent);
         }
     }
 
@@ -239,7 +246,7 @@ public String toString() {
         return "RequestContext [requestId=" + requestId + ", callerAppId=" + callerAppId + ", callerOrgMembership="
                 + callerOrgMembership + ", callerEnrolledStudies=" + callerEnrolledStudies + ", orgSponsoredStudies="
                 + orgSponsoredStudies + ", callerRoles=" + callerRoles + ", callerUserId=" + callerUserId
-                + ", callerClientInfo=" + callerClientInfo + ", callerIpAddress=" + callerIpAddress
-                + ", callerLanguages=" + callerLanguages + ", metrics=" + metrics + "]";
+                + ", callerIpAddress=" + callerIpAddress + ", callerLanguages=" + callerLanguages + ", metrics="
+                + metrics + ", userAgent=" + userAgent + "]";
     }
 }
diff --git a/src/main/java/org/sagebionetworks/bridge/dao/UploadDao.java b/src/main/java/org/sagebionetworks/bridge/dao/UploadDao.java
@@ -88,4 +88,7 @@ void writeValidationStatus(@Nonnull Upload upload, @Nonnull UploadStatus status,
      * @return a list of upload IDs to be deleted
      */
     List<String> deleteUploadsForHealthCode(@Nonnull String healthCode);
+
+    /** Update an upload record. */
+    void updateUpload(Upload upload);
 }
diff --git a/src/main/java/org/sagebionetworks/bridge/dynamodb/DynamoHealthDataRecordEx3.java b/src/main/java/org/sagebionetworks/bridge/dynamodb/DynamoHealthDataRecordEx3.java
@@ -40,6 +40,7 @@ public class DynamoHealthDataRecordEx3 implements HealthDataRecordEx3 {
     private Map<String, ExportedRecordInfo> exportedStudyRecords;
     private Map<String, String> metadata;
     private SharingScope sharingScope;
+    private String userAgent;
     private Long version;
     private String downloadUrl;
     private long downloadExpiration;
@@ -222,6 +223,16 @@ public void setSharingScope(SharingScope sharingScope) {
         this.sharingScope = sharingScope;
     }
 
+    @Override
+    public String getUserAgent() {
+        return userAgent;
+    }
+
+    @Override
+    public void setUserAgent(String userAgent) {
+        this.userAgent = userAgent;
+    }
+
     @DynamoDBVersionAttribute
     @Override
     public Long getVersion() {

diff --git a/src/main/java/org/sagebionetworks/bridge/dynamodb/DynamoUpload2.java b/src/main/java/org/sagebionetworks/bridge/dynamodb/DynamoUpload2.java
@@ -50,6 +50,7 @@ public class DynamoUpload2 implements Upload {
     private UploadCompletionClient completedBy;
     private LocalDate uploadDate;
     private String uploadId;
+    private String userAgent;
     private final List<String> validationMessageList = new ArrayList<>();
     private Long version;
     private Boolean zipped;
@@ -309,6 +310,16 @@ public void setUploadId(String uploadId) {
         this.uploadId = uploadId;
     }
 
+    @Override
+    public String getUserAgent() {
+        return userAgent;
+    }
+
+    @Override
+    public void setUserAgent(String userAgent) {
+        this.userAgent = userAgent;
+    }
+
     /**
      * Returns an immutable copy of the validation message list. Changes to the underlying message list will not be
      * reflected in this copy. Note that the validation message list will be non-null and will not contain null

diff --git a/src/main/java/org/sagebionetworks/bridge/dynamodb/DynamoUploadDao.java b/src/main/java/org/sagebionetworks/bridge/dynamodb/DynamoUploadDao.java
@@ -45,7 +45,6 @@
 import org.sagebionetworks.bridge.exceptions.BadRequestException;
 import org.sagebionetworks.bridge.exceptions.BridgeServiceException;
 import org.sagebionetworks.bridge.exceptions.ConcurrentModificationException;
-import org.sagebionetworks.bridge.exceptions.EntityNotFoundException;
 import org.sagebionetworks.bridge.exceptions.NotFoundException;
 import org.sagebionetworks.bridge.json.BridgeObjectMapper;
 import org.sagebionetworks.bridge.time.DateUtils;
@@ -333,5 +332,9 @@ public List<String> deleteUploadsForHealthCode(@Nonnull String healthCode) {
 
         return uploadIdList;
     }
-}
 
+    @Override
+    public void updateUpload(Upload upload) {
+        mapper.save(upload);
+    }
+}
diff --git a/src/main/java/org/sagebionetworks/bridge/models/healthdata/HealthDataRecordEx3.java b/src/main/java/org/sagebionetworks/bridge/models/healthdata/HealthDataRecordEx3.java
@@ -129,6 +129,10 @@ static HealthDataRecordEx3 createFromUpload(Upload upload) {
     SharingScope getSharingScope();
     void setSharingScope(SharingScope sharingScope);
 
+    /** Participant's User-Agent header. */
+    String getUserAgent();
+    void setUserAgent(String userAgent);
+
     /**
      * Record version. This is used to detect concurrency conflicts. For creating new health data records, this field
      * should be left unspecified. For updating records, this field should match the version of the most recent GET

diff --git a/src/main/java/org/sagebionetworks/bridge/models/upload/Upload.java b/src/main/java/org/sagebionetworks/bridge/models/upload/Upload.java
@@ -120,6 +120,10 @@ static Upload create() {
     /** @see #getUploadId */
     void setUploadId(String uploadId);
 
+    /** Participant's User-Agent header. */
+    String getUserAgent();
+    void setUserAgent(String userAgent);
+
     /** True if the upload is zipped. False if it is a single file. */
     boolean isZipped();
     void setZipped(boolean zipped);

diff --git a/src/main/java/org/sagebionetworks/bridge/services/Exporter3Service.java b/src/main/java/org/sagebionetworks/bridge/services/Exporter3Service.java
@@ -844,30 +844,38 @@ public void completeUpload(App app, Upload upload) throws JsonProcessingExceptio
         SharingScope sharingScope = account.getSharingScope();
         record.setSharingScope(sharingScope);
 
-        // Handle Client Info.
+        // Handle Client Info and User Agent.
         String clientInfoJsonText;
-        if (upload.getClientInfo() != null) {
+        String userAgent;
+        if (upload.getUserAgent() != null) {
             clientInfoJsonText = upload.getClientInfo();
+            userAgent = upload.getUserAgent();
         } else {
-            ClientInfo clientInfo = ClientInfo.UNKNOWN_CLIENT;
+            ClientInfo clientInfo;
             RequestContext requestContext = RequestContext.get();
             String calledUserId = requestContext.getCallerUserId();
             String uploaderUserId = account.getId();
             if (Objects.equals(calledUserId, uploaderUserId)) {
                 // The caller is the uploader. Get the Client Info from the RequestContext.
                 clientInfo = requestContext.getCallerClientInfo();
+                userAgent = requestContext.getUserAgent();
             } else {
                 // The caller is _not_ the uploader. Get the Client Info from the RequestInfoService.
                 RequestInfo requestInfo = requestInfoService.getRequestInfo(uploaderUserId);
                 if (requestInfo != null && requestInfo.getClientInfo() != null) {
                     clientInfo = requestInfo.getClientInfo();
+                    userAgent = requestInfo.getUserAgent();
+                } else {
+                    clientInfo = ClientInfo.UNKNOWN_CLIENT;
+                    userAgent = null;
                 }
             }
 
             clientInfoJsonText = BridgeObjectMapper.get().writerWithDefaultPrettyPrinter()
                     .writeValueAsString(clientInfo);
         }
         record.setClientInfo(clientInfoJsonText);
+        record.setUserAgent(userAgent);
 
         // Also mark with the latest participant version.
         Optional<ParticipantVersion> participantVersion = participantVersionService

diff --git a/src/main/java/org/sagebionetworks/bridge/services/UploadService.java b/src/main/java/org/sagebionetworks/bridge/services/UploadService.java
@@ -242,7 +242,8 @@ public UploadSession createUpload(String appId, StudyParticipant participant, Up
             uploadId = upload.getUploadId();
 
             // Get client info from Request Context, write it to the upload as JSON.
-            ClientInfo clientInfo = RequestContext.get().getCallerClientInfo();
+            RequestContext requestContext = RequestContext.get();
+            ClientInfo clientInfo = requestContext.getCallerClientInfo();
             try {
                 String clientInfoJsonText = BridgeObjectMapper.get().writerWithDefaultPrettyPrinter()
                         .writeValueAsString(clientInfo);
@@ -253,6 +254,13 @@ public UploadSession createUpload(String appId, StudyParticipant participant, Up
                         participant.getHealthCode(), ex);
             }
 
+            // Also, get the User Agent.
+            String userAgent = requestContext.getUserAgent();
+            upload.setUserAgent(userAgent);
+
+            // Write the upload back to the upload table with the user agent and client info.
+            uploadDao.updateUpload(upload);
+
             if (originalUploadId != null) {
                 // We had a dupe of a previous completed upload. Log this for future analysis.
                 logger.info("Detected dupe: App " + appId + ", upload " + uploadId +

diff --git a/src/main/java/org/sagebionetworks/bridge/spring/filters/RequestFilter.java b/src/main/java/org/sagebionetworks/bridge/spring/filters/RequestFilter.java
@@ -104,7 +104,7 @@ public void doFilter(ServletRequest req, ServletResponse res, FilterChain chain)
         RequestContext.Builder builder = new RequestContext.Builder()
                 .withRequestId(requestId)
                 .withCallerIpAddress(parseIpAddress(getRemoteAddress(request)))
-                .withCallerClientInfo(getClientInfoFromUserAgentHeader(request, response))
+                .withUserAgent(getUserAgentHeader(request, response))
                 .withCallerLanguages(getLanguagesFromAcceptLanguageHeader(request, response));
         setRequestContext(builder.build());
 
@@ -172,17 +172,19 @@ static List<String> getLanguagesFromAcceptLanguageHeader(HttpServletRequest requ
         return ImmutableList.of();
     }
 
-    static ClientInfo getClientInfoFromUserAgentHeader(HttpServletRequest request, HttpServletResponse response) {
+    static String getUserAgentHeader(HttpServletRequest request, HttpServletResponse response) {
         String userAgentHeader = request.getHeader(USER_AGENT);
-        ClientInfo info = ClientInfo.fromUserAgentCache(userAgentHeader);
 
         // if the user agent cannot be parsed (probably due to missing user agent string or unrecognizable user agent),
         // should set an extra header to http response as warning - we should have an user agent info for filtering to work
+        ClientInfo info = ClientInfo.fromUserAgentCache(userAgentHeader);
         if (info.equals(UNKNOWN_CLIENT)) {
             addWarningMessage(response, WARN_NO_USER_AGENT);
         }
         LOG.debug("User-Agent: '"+userAgentHeader+"' converted to " + info);    
-        return info;
+
+        // Return the User-Agent header value.
+        return userAgentHeader;
     }
 
     /**