Merge pull request #432 from RADAR-base/cacheQuestionnaires

Cache questionnaires and standardize github client access

.github/workflows/codeql-analysis.yml

@@ -41,7 +41,16 @@ jobs:
     - name: Checkout repository
       uses: actions/checkout@v3
 
-    # Initializes the CodeQL tools for scanning.
+    - uses: actions/setup-java@v3
+      if: matrix.language == 'java'
+      with:
+        java-version: 17
+        distribution: temurin
+
+    - uses: gradle/gradle-build-action@v2
+      if: matrix.language == 'java'
+
+      # Initializes the CodeQL tools for scanning.
     - name: Initialize CodeQL
       uses: github/codeql-action/init@v2
       with:

.gitignore

@@ -45,4 +45,5 @@ bin/
 
 docker/prod
 
-*__pycache__
+*__pycache__
+google-credentials.json

src/integrationTest/resources/application.properties

@@ -96,3 +96,9 @@ security.radar.managementportal.url=http://localhost:8081
 
 # Github Authentication
 security.github.client.token=
+security.github.client.timeout=10
+# max content size 1 MB
+security.github.client.maxContentLength=1000000
+security.github.cache.size=10000
+security.github.cache.duration=3600
+security.github.cache.retryDuration=60

src/integrationTest/resources/docker/docker-compose.yml

@@ -64,7 +64,7 @@ services:
   # Management Portal                                                         #
   #---------------------------------------------------------------------------#
   managementportal:
-    image: radarbase/management-portal:0.5.6
+    image: radarbase/management-portal:2.0.0
     ports:
       - "8081:8081"
     environment:

src/main/java/org/radarbase/appserver/controller/GithubEndpoint.java

@@ -22,7 +22,7 @@
 package org.radarbase.appserver.controller;
 
 import org.radarbase.appserver.config.AuthConfig;
-import org.radarbase.appserver.service.GithubClient;
+import org.radarbase.appserver.service.GithubService;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.ResponseEntity;
@@ -31,17 +31,16 @@
 import org.springframework.web.bind.annotation.RestController;
 import radar.spring.auth.common.Authorized;
 
-import java.io.IOException;
 import java.net.MalformedURLException;
 
 @RestController
 public class GithubEndpoint {
 
-    private transient GithubClient githubClient;
+    private final transient GithubService githubService;
 
     @Autowired
-    public GithubEndpoint(GithubClient githubClient) {
-        this.githubClient = githubClient;
+    public GithubEndpoint(GithubService githubService) {
+        this.githubService = githubService;
     }
 
     @Authorized(
@@ -51,13 +50,13 @@ public GithubEndpoint(GithubClient githubClient) {
             PathsUtil.GITHUB_PATH
             + "/" +
             PathsUtil.GITHUB_CONTENT_PATH)
-    public ResponseEntity getGithubContent(@RequestParam() String url
+    public ResponseEntity<String> getGithubContent(@RequestParam() String url
     ) {
         try {
-            return ResponseEntity.ok().body(this.githubClient.getGithubContent(url));
+            return ResponseEntity.ok().body(this.githubService.getGithubContent(url));
         } catch (MalformedURLException e) {
             return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(e.getMessage());
-        } catch (IOException | InterruptedException e) {
+        } catch (Exception e) {
             return ResponseEntity.status(HttpStatus.BAD_GATEWAY).body("Error getting content from Github.");
         }
     }

src/main/java/org/radarbase/appserver/entity/User.java

@@ -22,23 +22,36 @@
 package org.radarbase.appserver.entity;
 
 import com.fasterxml.jackson.annotation.JsonIgnore;
-
-import java.io.Serializable;
-import java.time.Instant;
-import java.util.HashMap;
-import java.util.Map;
-import java.util.Objects;
-import jakarta.persistence.*;
+import jakarta.persistence.CascadeType;
+import jakarta.persistence.CollectionTable;
+import jakarta.persistence.Column;
+import jakarta.persistence.ElementCollection;
+import jakarta.persistence.Entity;
+import jakarta.persistence.FetchType;
+import jakarta.persistence.GeneratedValue;
+import jakarta.persistence.GenerationType;
+import jakarta.persistence.Id;
+import jakarta.persistence.JoinColumn;
+import jakarta.persistence.ManyToOne;
+import jakarta.persistence.MapKeyColumn;
+import jakarta.persistence.OneToOne;
+import jakarta.persistence.Table;
+import jakarta.persistence.UniqueConstraint;
 import jakarta.validation.constraints.NotEmpty;
 import jakarta.validation.constraints.NotNull;
-
 import lombok.Getter;
 import lombok.ToString;
 import org.hibernate.annotations.OnDelete;
 import org.hibernate.annotations.OnDeleteAction;
 import org.radarbase.appserver.dto.fcm.FcmUserDto;
 import org.springframework.lang.Nullable;
 
+import java.io.Serializable;
+import java.time.Instant;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Objects;
+
 /**
  * {@link Entity} for persisting users. The corresponding DTO is {@link FcmUserDto}. A {@link
  * Project} can have multiple {@link User} (Many-to-One).
@@ -97,7 +110,7 @@ public class User extends AuditModel implements Serializable {
     @CollectionTable(name = "attributes_map")
     @MapKeyColumn(name = "key", nullable = true)
     @Column(name = "value")
-    private Map<String, String> attributes = new HashMap<String, String>();
+    private Map<String, String> attributes = new HashMap<>();
 
     public User setSubjectId(String subjectId) {
         this.subjectId = subjectId;

src/main/java/org/radarbase/appserver/service/GithubClient.java

@@ -21,7 +21,7 @@
 
 package org.radarbase.appserver.service;
 
-import com.fasterxml.jackson.databind.ObjectMapper;
+import jakarta.annotation.Nonnull;
 import lombok.SneakyThrows;
 import lombok.extern.slf4j.Slf4j;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -33,6 +33,7 @@
 import org.springframework.web.server.ResponseStatusException;
 
 import java.io.IOException;
+import java.io.InputStream;
 import java.net.MalformedURLException;
 import java.net.URI;
 import java.net.http.HttpClient;
@@ -44,53 +45,94 @@
 @Component
 @Scope(value = ConfigurableBeanFactory.SCOPE_SINGLETON)
 public class GithubClient {
-
     private static final String GITHUB_API_URI = "api.github.com";
     private static final String GITHUB_API_ACCEPT_HEADER = "application/vnd.github.v3+json";
-    private static final String LOCATION_HEADER = "location";
-    private final transient ObjectMapper objectMapper;
     private final transient HttpClient client;
 
-    @Value("${security.github.client.token}")
-    private transient String githubToken;
+    @Nonnull
+    private final transient String authorizationHeader;
+
+    private transient final Duration httpTimeout;
+
+    @Value("${security.github.client.maxContentLength:1000000}")
+    private transient int maxContentLength;
 
     @SneakyThrows
     @Autowired
-    public GithubClient(ObjectMapper objectMapper) {
-        this.objectMapper = objectMapper;
-        client = HttpClient.newBuilder().followRedirects(HttpClient.Redirect.NORMAL).connectTimeout(Duration.ofSeconds(10)).build();
-    }
-
-    private static boolean isSuccessfulResponse(HttpResponse response) {
-        return response.statusCode() >= 200 && response.statusCode() < 300;
+    public GithubClient(
+            @Value("${security.github.client.timeout:10}") int httpTimeout,
+            @Value("${security.github.client.token:}") String githubToken) {
+        this.authorizationHeader = githubToken != null ? "Bearer " + githubToken.trim() : "";
+        this.httpTimeout = Duration.ofSeconds(httpTimeout);
+        this.client = HttpClient.newBuilder()
+                .followRedirects(HttpClient.Redirect.NORMAL)
+                .connectTimeout(this.httpTimeout)
+                .build();
     }
 
     public String getGithubContent(String url) throws IOException, InterruptedException {
         URI uri = URI.create(url);
         if (!this.isValidGithubUri(uri)) {
             throw new MalformedURLException("Invalid Github url.");
         }
-        HttpResponse response = client.send(getRequest(uri), HttpResponse.BodyHandlers.ofString());
-        if (isSuccessfulResponse(response)) {
-            return response.body().toString();
+        HttpResponse<InputStream> response = makeRequest(uri);
+
+        if (response.statusCode() >= 200 && response.statusCode() < 300) {
+            checkContentLengthHeader(response);
+
+            try (InputStream inputStream = response.body()) {
+                byte[] bytes = inputStream.readNBytes(maxContentLength + 1);
+                checkContentLength(bytes.length);
+                return new String(bytes);
+            }
         } else {
             log.error("Error getting Github content from URL {} : {}", url, response);
             throw new ResponseStatusException(
                     HttpStatus.valueOf(response.statusCode()), "Github content could not be retrieved");
         }
     }
 
+    private HttpResponse<InputStream> makeRequest(URI uri) throws InterruptedException {
+        try {
+            return client.send(getRequest(uri), HttpResponse.BodyHandlers.ofInputStream());
+        } catch (IOException ex) {
+            log.error("Failed to retrieve data from github: {}", ex.toString());
+            throw new ResponseStatusException(HttpStatus.BAD_GATEWAY, "Github responded with an error.");
+        }
+    }
+
+    private void checkContentLengthHeader(HttpResponse<?> response) {
+        response.headers().firstValue("Content-Length")
+                .map((l) -> {
+                    try {
+                        return Integer.valueOf(l);
+                    } catch (NumberFormatException ex) {
+                        return null;
+                    }
+                })
+                .ifPresent(this::checkContentLength);
+    }
+
+    private void checkContentLength(int contentLength) {
+        if (contentLength > maxContentLength) {
+            throw new ResponseStatusException(
+                    HttpStatus.BAD_REQUEST, "Github content is too large");
+        }
+    }
+
     public boolean isValidGithubUri(URI uri) {
-        return uri.getHost().contains(GITHUB_API_URI);
+        return uri.getHost().equalsIgnoreCase(GITHUB_API_URI)
+                && uri.getScheme().equalsIgnoreCase("https")
+                && (uri.getPort() == -1 || uri.getPort() == 443);
     }
 
     private HttpRequest getRequest(URI uri) {
         HttpRequest.Builder request = HttpRequest.newBuilder(uri)
                 .header("Accept", GITHUB_API_ACCEPT_HEADER)
                 .GET()
-                .timeout(Duration.ofSeconds(10));
-        if (githubToken != null && !githubToken.isEmpty()) {
-            request.header("Authorization", "Bearer " + githubToken);
+                .timeout(httpTimeout);
+        if (!authorizationHeader.isEmpty()) {
+            request.header("Authorization", authorizationHeader);
         }
         return request.build();
     }

src/main/java/org/radarbase/appserver/service/GithubService.java

@@ -0,0 +1,36 @@
+package org.radarbase.appserver.service;
+
+import org.radarbase.appserver.util.CachedFunction;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.beans.factory.annotation.Value;
+import org.springframework.beans.factory.config.ConfigurableBeanFactory;
+import org.springframework.context.annotation.Scope;
+import org.springframework.stereotype.Component;
+
+import java.time.Duration;
+
+@Component
+@Scope(value = ConfigurableBeanFactory.SCOPE_SINGLETON)
+public class GithubService {
+
+    private final transient CachedFunction<String, String> cachedGetContent;
+
+    @Autowired
+    public GithubService(
+            GithubClient githubClient,
+            @Value("${security.github.cache.duration:3600}")
+            int cacheTime,
+            @Value("${security.github.cache.retryDuration:60}")
+            int retryTime,
+            @Value("${security.github.cache.size:10000}")
+            int maxSize) {
+        this.cachedGetContent = new CachedFunction<>(githubClient::getGithubContent,
+                Duration.ofSeconds(cacheTime),
+                Duration.ofSeconds(retryTime),
+                maxSize);
+    }
+
+    public String getGithubContent(String url) throws Exception {
+        return this.cachedGetContent.applyWithException(url);
+    }
+}

src/main/java/org/radarbase/appserver/service/questionnaire/protocol/DefaultProtocolGenerator.java

@@ -21,22 +21,20 @@
 
 package org.radarbase.appserver.service.questionnaire.protocol;
 
-import java.io.IOException;
-import java.time.Duration;
-import java.util.List;
-import java.util.Map;
-import java.util.NoSuchElementException;
-
 import lombok.NonNull;
 import lombok.extern.slf4j.Slf4j;
 import org.radarbase.appserver.dto.protocol.Protocol;
-import org.radarbase.appserver.entity.User;
 import org.radarbase.appserver.util.CachedMap;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.config.ConfigurableBeanFactory;
 import org.springframework.context.annotation.Scope;
 import org.springframework.stereotype.Service;
 
+import java.io.IOException;
+import java.time.Duration;
+import java.util.Map;
+import java.util.NoSuchElementException;
+
 /**
  * @author yatharthranjan
  * @see <a href="https://github.com/RADAR-base/RADAR-aRMT-protocols">aRMT Protocols</a>
@@ -90,7 +88,7 @@ public Protocol getProtocol(String projectId) throws IOException {
             return cachedProjectProtocolMap.get(projectId);
         } catch (IOException ex) {
             log.warn(
-                    "Cannot retrieve Protocols for project {} : {}, Using cached values.", projectId, ex);
+                    "Cannot retrieve Protocols for project {} : {}, Using cached values.", projectId, ex.toString());
             return cachedProjectProtocolMap.get(true).get(projectId);
         }
     }
@@ -115,7 +113,7 @@ public Protocol getProtocolForSubject(String subjectId) {
             return protocol;
         } catch (IOException ex) {
             log.warn(
-                    "Cannot retrieve Protocols for subject {} : {}, Using cached values.", subjectId, ex);
+                    "Cannot retrieve Protocols for subject {} : {}, Using cached values.", subjectId, ex.toString());
             return cachedProtocolMap.getCache().get(subjectId);
         } catch(NoSuchElementException ex) {
             log.warn("Subject does not exist in map. Fetching..");