diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/olat/OlatLmsAPITemplate.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/olat/OlatLmsAPITemplate.java index 70f38b23..68cb949d 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/olat/OlatLmsAPITemplate.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/olat/OlatLmsAPITemplate.java @@ -75,6 +75,8 @@ public class OlatLmsAPITemplate extends AbstractCachedCourseAccess implements Lm private final APITemplateDataSupplier apiTemplateDataSupplier; private final Long lmsSetupId; + private OlatLmsRestTemplate cachedRestTemplate; + protected OlatLmsAPITemplate( final ClientHttpRequestFactoryService clientHttpRequestFactoryService, final ClientCredentialService clientCredentialService, @@ -112,9 +114,8 @@ public class OlatLmsAPITemplate extends AbstractCachedCourseAccess implements Lm if (testLmsSetupSettings.hasAnyError()) { return testLmsSetupSettings; } - // TODO: improve error handling try { - final RestTemplate restTemplate = this.getRestTemplate().get(); + this.getRestTemplate().get(); } catch (Exception e) { return LmsSetupTestResult.ofQuizAccessAPIError(LmsType.OPEN_OLAT, "Unspecific error connecting to OLAT API"); @@ -124,20 +125,7 @@ public class OlatLmsAPITemplate extends AbstractCachedCourseAccess implements Lm @Override public LmsSetupTestResult testCourseRestrictionAPI() { - // TODO: Any reason to implement a separate check or is this good enough? return testCourseAccessAPI(); - - /*final LmsSetupTestResult testLmsSetupSettings = testLmsSetupSettings(); - if (testLmsSetupSettings.hasAnyError()) { - return testLmsSetupSettings; - } - - if (LmsType.OPEN_OLAT.features.contains(Features.SEB_RESTRICTION)) { - - } - - return LmsSetupTestResult.ofOkay(LmsType.OPEN_OLAT); - */ } private LmsSetupTestResult testLmsSetupSettings() { @@ -233,10 +221,9 @@ public class OlatLmsAPITemplate extends AbstractCachedCourseAccess implements Lm }; } - private String examUrl(long olatTestId) { + private String examUrl(long olatRepositoryId) { final LmsSetup lmsSetup = this.apiTemplateDataSupplier.getLmsSetup(); - // TODO: at the moment, we don't know olatTestId because we get the assessment mode id (a.key), not the test id. - return lmsSetup.lmsApiUrl + "/auth/RepositoryEntry/" + olatTestId; + return lmsSetup.lmsApiUrl + "/auth/RepositoryEntry/" + olatRepositoryId; } private List collectAllQuizzes(final OlatLmsRestTemplate restTemplate, final FilterMap filterMap) { @@ -251,8 +238,9 @@ public class OlatLmsAPITemplate extends AbstractCachedCourseAccess implements Lm final List as = this.apiGetList(restTemplate, url, new ParameterizedTypeReference>(){}); return as.stream() - .map(a -> new QuizData( - String.format("%d", a.assessmentModeKey), + .map(a -> { + return new QuizData( + String.format("%d", a.key), lmsSetup.getInstitutionId(), lmsSetup.id, lmsSetup.getLmsType(), @@ -261,7 +249,7 @@ public class OlatLmsAPITemplate extends AbstractCachedCourseAccess implements Lm Utils.toDateTimeUTC(a.dateFrom), Utils.toDateTimeUTC(a.dateTo), examUrl(a.repositoryEntryKey), - new HashMap())) + new HashMap());}) .collect(Collectors.toList()); } @@ -283,7 +271,7 @@ public class OlatLmsAPITemplate extends AbstractCachedCourseAccess implements Lm final String url = String.format("/restapi/assessment_modes/%s", id); final AssessmentData a = this.apiGet(restTemplate, url, AssessmentData.class); return new QuizData( - String.format("%d", a.assessmentModeKey), + String.format("%d", a.key), lmsSetup.getInstitutionId(), lmsSetup.id, lmsSetup.getLmsType(), @@ -303,8 +291,7 @@ public class OlatLmsAPITemplate extends AbstractCachedCourseAccess implements Lm String.valueOf(u.key), u.lastName + ", " + u.firstName, u.username, - // TODO: other placeholder value? null? - "OLAT does not provide email addresses", + "OLAT API does not provide email addresses", attrs); } @@ -348,18 +335,16 @@ public class OlatLmsAPITemplate extends AbstractCachedCourseAccess implements Lm @Override public Result getSEBClientRestriction(final Exam exam) { - return Result.of(getRestTemplate() - .map(t -> this.getRestrictionForAssignmentId(t, exam.externalId)) - .getOrThrow()); + return getRestTemplate() + .map(t -> this.getRestrictionForAssignmentId(t, exam.externalId)); } @Override public Result applySEBClientRestriction( final String externalExamId, final SEBRestriction sebRestrictionData) { - return Result.of(getRestTemplate() - .map(t -> this.setRestrictionForAssignmentId(t, externalExamId, sebRestrictionData)) - .getOrThrow()); + return getRestTemplate() + .map(t -> this.setRestrictionForAssignmentId(t, externalExamId, sebRestrictionData)); } @Override @@ -367,10 +352,9 @@ public class OlatLmsAPITemplate extends AbstractCachedCourseAccess implements Lm @SuppressWarnings("unused") final String quizId = exam.externalId; - return Result.of(getRestTemplate() + return getRestTemplate() .map(t -> this.deleteRestrictionForAssignmentId(t, exam.externalId)) - .map(x -> exam) - .getOrThrow()); + .map(x -> exam); } private T apiGet(final RestTemplate restTemplate, String url, Class type) { @@ -417,29 +401,33 @@ public class OlatLmsAPITemplate extends AbstractCachedCourseAccess implements Lm } private Result getRestTemplate() { - // TODO: cache/reuse authenticated template for more than 1 request? - final LmsSetup lmsSetup = this.apiTemplateDataSupplier.getLmsSetup(); - final ClientCredentials credentials = this.apiTemplateDataSupplier.getLmsClientCredentials(); - final ProxyData proxyData = this.apiTemplateDataSupplier.getProxyData(); + return Result.tryCatch(() -> { + if (this.cachedRestTemplate != null) { return this.cachedRestTemplate; } + final LmsSetup lmsSetup = this.apiTemplateDataSupplier.getLmsSetup(); + final ClientCredentials credentials = this.apiTemplateDataSupplier.getLmsClientCredentials(); + final ProxyData proxyData = this.apiTemplateDataSupplier.getProxyData(); - final CharSequence plainClientId = credentials.clientId; - final CharSequence plainClientSecret = this.clientCredentialService - .getPlainClientSecret(credentials) - .getOrThrow(); + final CharSequence plainClientId = credentials.clientId; + final CharSequence plainClientSecret = this.clientCredentialService + .getPlainClientSecret(credentials) + .getOrThrow(); - final ClientCredentialsResourceDetails details = new ClientCredentialsResourceDetails(); - details.setAccessTokenUri(lmsSetup.lmsApiUrl + "/restapi/auth/"); - details.setClientId(plainClientId.toString()); - details.setClientSecret(plainClientSecret.toString()); + final ClientCredentialsResourceDetails details = new ClientCredentialsResourceDetails(); + details.setAccessTokenUri(lmsSetup.lmsApiUrl + "/restapi/auth/"); + details.setClientId(plainClientId.toString()); + details.setClientSecret(plainClientSecret.toString()); - final ClientHttpRequestFactory clientHttpRequestFactory = this.clientHttpRequestFactoryService - .getClientHttpRequestFactory(proxyData) - .getOrThrow(); + final ClientHttpRequestFactory clientHttpRequestFactory = this.clientHttpRequestFactoryService + .getClientHttpRequestFactory(proxyData) + .getOrThrow(); - final OlatLmsRestTemplate template = new OlatLmsRestTemplate(details); - template.setRequestFactory(clientHttpRequestFactory); + final OlatLmsRestTemplate template = new OlatLmsRestTemplate(details); + template.setRequestFactory(clientHttpRequestFactory); - return Result.of(template); + this.cachedRestTemplate = template; + + return this.cachedRestTemplate; + }); } } diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/olat/OlatLmsData.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/olat/OlatLmsData.java index 5698080b..9f7dff08 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/olat/OlatLmsData.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/olat/OlatLmsData.java @@ -23,12 +23,12 @@ public final class OlatLmsData { "dateFrom": 1624420800000, "dateTo": 1624658400000, "description": "", - "assessmentModeKey": 6356992, + "key": 6356992, “repositoryEntryKey”: 462324, "name": "SEB test" } */ - public long assessmentModeKey; + public long key; public long repositoryEntryKey; public String name; public String description; diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/olat/OlatLmsRestTemplate.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/olat/OlatLmsRestTemplate.java index 65112dd6..18e2a7a0 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/olat/OlatLmsRestTemplate.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/olat/OlatLmsRestTemplate.java @@ -16,6 +16,7 @@ import org.slf4j.LoggerFactory; import org.springframework.web.client.RestTemplate; import org.springframework.security.oauth2.client.token.grant.client.ClientCredentialsResourceDetails; import org.springframework.http.HttpEntity; +import org.springframework.http.HttpStatus; import org.springframework.http.HttpRequest; import org.springframework.http.HttpHeaders; import org.springframework.http.ResponseEntity; @@ -25,35 +26,53 @@ import org.springframework.http.client.ClientHttpResponse; public class OlatLmsRestTemplate extends RestTemplate { - private static final Logger log = LoggerFactory.getLogger(OlatLmsRestTemplate.class); + private static final Logger log = LoggerFactory.getLogger(OlatLmsRestTemplate.class); - public String token; + public String token; + private ClientCredentialsResourceDetails details; - public OlatLmsRestTemplate(ClientCredentialsResourceDetails details) { + public OlatLmsRestTemplate(ClientCredentialsResourceDetails details) { super(); - - // Authenticate with OLAT and store the received X-OLAT-TOKEN - final String authUrl = String.format("%s%s?password=%s", - details.getAccessTokenUri(), - details.getClientId(), - details.getClientSecret()); - final HttpHeaders httpHeaders = new HttpHeaders(); - httpHeaders.set("accept", "application/json"); - ResponseEntity response = this.getForEntity(authUrl, String.class); - HttpHeaders responseHeaders = response.getHeaders(); - log.debug("OLAT Auth Response Headers: {}", responseHeaders); - token = responseHeaders.getFirst("X-OLAT-TOKEN"); + this.details = details; + authenticate(); // Add X-OLAT-TOKEN request header to every request done using this RestTemplate this.getInterceptors().add(new ClientHttpRequestInterceptor(){ @Override public ClientHttpResponse intercept(HttpRequest request, byte[] body, ClientHttpRequestExecution execution) throws IOException { - request.getHeaders().set("X-OLAT-TOKEN", token); - request.getHeaders().set("accept", "application/json"); - HttpHeaders responseHeaders = response.getHeaders(); - return execution.execute(request, body); + request.getHeaders().set("accept", "application/json"); + // if we don't have a token (this is normal during authentication), just do the call + if (token == null) { return execution.execute(request, body); } + // otherwise, add the X-OLAT-TOKEN + request.getHeaders().set("X-OLAT-TOKEN", token); + ClientHttpResponse response = execution.execute(request, body); + log.debug("OLAT [regular API call] Response Headers: {}", response.getHeaders()); + // If we get a 401, re-authenticate and try once more + if (response.getStatusCode() == HttpStatus.UNAUTHORIZED) { + authenticate(); + request.getHeaders().set("X-OLAT-TOKEN", token); + response = execution.execute(request, body); + log.debug("OLAT [retry API call] Response Headers: {}", response.getHeaders()); + } + return response; } }); - } + } + + private void authenticate() { + // Authenticate with OLAT and store the received X-OLAT-TOKEN + token = null; + final String authUrl = String.format("%s%s?password=%s", + details.getAccessTokenUri(), + details.getClientId(), + details.getClientSecret()); + final HttpHeaders httpHeaders = new HttpHeaders(); + ResponseEntity response = this.getForEntity(authUrl, String.class); + HttpHeaders responseHeaders = response.getHeaders(); + log.debug("OLAT [authenticate] Response Headers: {}", responseHeaders); + token = responseHeaders.getFirst("X-OLAT-TOKEN"); + } + + }