From 270dc1bf3094268a2e7fa96c24d01380ecf7173a Mon Sep 17 00:00:00 2001 From: Martin Ledvinka Date: Mon, 16 Sep 2024 08:46:09 +0200 Subject: [PATCH] [Ref] Remove ConfigParam enum, use just constants. Fix Javadoc reference issues. --- .../cvut/kbss/termit/config/WebAppConfig.java | 17 ++--- .../util/AdjustedUriTemplateProxyServlet.java | 13 +++- .../cz/cvut/kbss/termit/util/ConfigParam.java | 45 ------------ .../cvut/kbss/termit/util/Configuration.java | 1 - .../cz/cvut/kbss/termit/util/Constants.java | 2 - .../termit/util/throttle/ThrottleAspect.java | 69 +++++++++++-------- 6 files changed, 61 insertions(+), 86 deletions(-) delete mode 100644 src/main/java/cz/cvut/kbss/termit/util/ConfigParam.java diff --git a/src/main/java/cz/cvut/kbss/termit/config/WebAppConfig.java b/src/main/java/cz/cvut/kbss/termit/config/WebAppConfig.java index eaa03f2e8..a7c4737fe 100644 --- a/src/main/java/cz/cvut/kbss/termit/config/WebAppConfig.java +++ b/src/main/java/cz/cvut/kbss/termit/config/WebAppConfig.java @@ -29,7 +29,6 @@ import cz.cvut.kbss.jsonld.jackson.serialization.SerializationConstants; import cz.cvut.kbss.termit.rest.servlet.DiagnosticsContextFilter; import cz.cvut.kbss.termit.util.AdjustedUriTemplateProxyServlet; -import cz.cvut.kbss.termit.util.ConfigParam; import cz.cvut.kbss.termit.util.Constants; import cz.cvut.kbss.termit.util.json.MultilingualStringDeserializer; import cz.cvut.kbss.termit.util.json.MultilingualStringSerializer; @@ -137,8 +136,10 @@ public ServletWrappingController sparqlEndpointController() throws Exception { final cz.cvut.kbss.termit.util.Configuration.Repository repository = config.getRepository(); p.setProperty("targetUri", repository.getUrl()); p.setProperty("log", "false"); - p.setProperty(ConfigParam.REPO_USERNAME.toString(), repository.getUsername() != null ? repository.getUsername() : ""); - p.setProperty(ConfigParam.REPO_PASSWORD.toString(), repository.getPassword() != null ? repository.getPassword() : ""); + p.setProperty(AdjustedUriTemplateProxyServlet.REPO_USERNAME_PARAM, + repository.getUsername() != null ? repository.getUsername() : ""); + p.setProperty(AdjustedUriTemplateProxyServlet.REPO_PASSWORD_PARAM, + repository.getPassword() != null ? repository.getPassword() : ""); controller.setInitParameters(p); controller.afterPropertiesSet(); return controller; @@ -149,7 +150,7 @@ public SimpleUrlHandlerMapping sparqlQueryControllerMapping() throws Exception { SimpleUrlHandlerMapping mapping = new SimpleUrlHandlerMapping(); mapping.setOrder(0); final Map urlMap = Collections.singletonMap(Constants.REST_MAPPING_PATH + "/query", - sparqlEndpointController()); + sparqlEndpointController()); mapping.setUrlMap(urlMap); return mapping; } @@ -195,10 +196,10 @@ public FilterRegistrationBean mdcFilter() { @Bean public OpenAPI customOpenAPI() { return new OpenAPI().components(new Components().addSecuritySchemes("bearer-key", - new SecurityScheme().type( - SecurityScheme.Type.HTTP) - .scheme("bearer") - .bearerFormat("JWT"))) + new SecurityScheme().type( + SecurityScheme.Type.HTTP) + .scheme("bearer") + .bearerFormat("JWT"))) .info(new Info().title("TermIt REST API").description("TermIt REST API definition.") .version(version)); } diff --git a/src/main/java/cz/cvut/kbss/termit/util/AdjustedUriTemplateProxyServlet.java b/src/main/java/cz/cvut/kbss/termit/util/AdjustedUriTemplateProxyServlet.java index 2355a18ea..436cf8397 100644 --- a/src/main/java/cz/cvut/kbss/termit/util/AdjustedUriTemplateProxyServlet.java +++ b/src/main/java/cz/cvut/kbss/termit/util/AdjustedUriTemplateProxyServlet.java @@ -38,12 +38,21 @@ */ public class AdjustedUriTemplateProxyServlet extends URITemplateProxyServlet { + /** + * Configuration parameter representing username to use when connecting to the repository. + */ + public static final String REPO_USERNAME_PARAM = "repository.username"; + /** + * Configuration parameter representing password to use when connecting to the repository. + */ + public static final String REPO_PASSWORD_PARAM = "repository.password"; + @Override protected void service(HttpServletRequest servletRequest, HttpServletResponse servletResponse) throws ServletException, IOException { - final String username = getConfigParam(ConfigParam.REPO_USERNAME.toString()); - final String password = getConfigParam(ConfigParam.REPO_PASSWORD.toString()); + final String username = getConfigParam(REPO_USERNAME_PARAM); + final String password = getConfigParam(REPO_PASSWORD_PARAM); super.service(new AuthenticatingServletRequestWrapper(servletRequest, username, password), new HttpServletResponseWrapper(servletResponse) { @Override diff --git a/src/main/java/cz/cvut/kbss/termit/util/ConfigParam.java b/src/main/java/cz/cvut/kbss/termit/util/ConfigParam.java deleted file mode 100644 index ed48145bf..000000000 --- a/src/main/java/cz/cvut/kbss/termit/util/ConfigParam.java +++ /dev/null @@ -1,45 +0,0 @@ -/* - * TermIt - * Copyright (C) 2023 Czech Technical University in Prague - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see . - */ -package cz.cvut.kbss.termit.util; - -/** - * Application configuration parameters, loaded from {@code application.yml} provided on classpath. - */ -public enum ConfigParam { - - /** - * Username for connecting to the application repository. - */ - REPO_USERNAME("repository.username"), - - /** - * Password for connecting to the application repository. - */ - REPO_PASSWORD("repository.password"); - - private final String parameter; - - ConfigParam(String parameter) { - this.parameter = parameter; - } - - @Override - public String toString() { - return parameter; - } -} diff --git a/src/main/java/cz/cvut/kbss/termit/util/Configuration.java b/src/main/java/cz/cvut/kbss/termit/util/Configuration.java index cf609cab8..b9be4a7db 100644 --- a/src/main/java/cz/cvut/kbss/termit/util/Configuration.java +++ b/src/main/java/cz/cvut/kbss/termit/util/Configuration.java @@ -80,7 +80,6 @@ public class Configuration { * After how much time, should objects with completed futures be discarded. * The value must be positive ({@code > 0}). * @configurationdoc.default 1 minute - * @see ThrottleAspect#clearOldFutures() */ private Duration throttleDiscardThreshold = Duration.ofMinutes(1); diff --git a/src/main/java/cz/cvut/kbss/termit/util/Constants.java b/src/main/java/cz/cvut/kbss/termit/util/Constants.java index 601c4703f..5d7ead6a9 100644 --- a/src/main/java/cz/cvut/kbss/termit/util/Constants.java +++ b/src/main/java/cz/cvut/kbss/termit/util/Constants.java @@ -18,12 +18,10 @@ package cz.cvut.kbss.termit.util; import cz.cvut.kbss.jopa.vocabulary.SKOS; -import cz.cvut.kbss.termit.util.throttle.ThrottleAspect; import org.springframework.data.domain.PageRequest; import org.springframework.data.domain.Pageable; import java.net.URI; -import java.time.Duration; import java.time.Instant; import java.time.ZoneId; import java.time.format.DateTimeFormatter; diff --git a/src/main/java/cz/cvut/kbss/termit/util/throttle/ThrottleAspect.java b/src/main/java/cz/cvut/kbss/termit/util/throttle/ThrottleAspect.java index fcf8ea14b..f86ab39d5 100644 --- a/src/main/java/cz/cvut/kbss/termit/util/throttle/ThrottleAspect.java +++ b/src/main/java/cz/cvut/kbss/termit/util/throttle/ThrottleAspect.java @@ -56,8 +56,8 @@ import static org.springframework.beans.factory.config.ConfigurableBeanFactory.SCOPE_SINGLETON; /** - * @see Throttle * @implNote The aspect is configured in {@code spring-aop.xml}, this uses Spring AOP instead of AspectJ. + * @see Throttle */ @Order @Scope(SCOPE_SINGLETON) @@ -80,22 +80,21 @@ public class ThrottleAspect extends LongRunningTaskScheduler { /** * The last run is updated every time a task is finished. + * * @implSpec Synchronize in the field declaration order before modification */ private final Map lastRun; /** - * Scheduled futures are returned from {@link #taskScheduler}. - * Futures are completed by execution of tasks created in {@link #createRunnableToSchedule}. - * Records about them are used for their cancellation in case of debouncing. + * Scheduled futures are returned from {@link #taskScheduler}. Futures are completed by execution of tasks created + * in {@link #createRunnableToSchedule}. Records about them are used for their cancellation in case of debouncing. * * @implSpec Synchronize in the field declaration order before modification */ private final NavigableMap> scheduledFutures; /** - * Thread safe set holding identifiers of threads that are - * currently executing a throttled task. + * Thread safe set holding identifiers of threads that are currently executing a throttled task. */ private final Set throttledThreads = ConcurrentHashMap.newKeySet(); @@ -113,6 +112,7 @@ public class ThrottleAspect extends LongRunningTaskScheduler { /** * Used for acquiring {@link #lastRun} timestamps. + * * @implNote for testing purposes */ private final Clock clock; @@ -123,8 +123,8 @@ public class ThrottleAspect extends LongRunningTaskScheduler { private final SynchronousTransactionExecutor transactionExecutor; /** - * A timestamp of the last time maps were cleaned. - * The reference might be null. + * A timestamp of the last time maps were cleaned. The reference might be null. + * * @see #clearOldFutures() */ private final AtomicReference lastClear; @@ -185,7 +185,8 @@ private static StandardEvaluationContext makeDefaultContext() { /** * @return future or null * @throws TermItException when the target method throws - * @throws IllegalCallerException when the annotated method returns another type than {@code void}, {@link Void} or {@link Future} + * @throws IllegalCallerException when the annotated method returns another type than {@code void}, {@link Void} or + * {@link Future} * @implNote Around advice configured in {@code spring-aop.xml} */ public @Nullable Object throttleMethodCall(@NonNull ProceedingJoinPoint joinPoint, @@ -265,8 +266,8 @@ private static StandardEvaluationContext makeDefaultContext() { if (oldScheduledFuture == null || oldThrottledFuture != future || oldScheduledFuture.isDone()) { boolean oldFutureIsDone = oldScheduledFuture == null || oldScheduledFuture.isDone(); if (oldThrottledFuture != future) { - oldThrottledFuture.then(ignored -> - schedule(identifier, pair.getFirst(), throttleExpired && oldFutureIsDone) + oldThrottledFuture.then(ignored -> schedule(identifier, pair.getFirst(), + throttleExpired && oldFutureIsDone) ); } else { schedule(identifier, pair.getFirst(), throttleExpired && oldFutureIsDone); @@ -352,11 +353,13 @@ private Pair> getFutureTask(@NonNull Proceedin } }, List.of()); } else { - throw new ThrottleAspectException("Invalid return type for " + joinPoint.getSignature() + " annotated with @Debounce, only Future or void allowed!"); + throw new ThrottleAspectException( + "Invalid return type for " + joinPoint.getSignature() + " annotated with @Debounce, only Future or void allowed!"); } final boolean withTransaction = methodSignature.getMethod() != null && methodSignature.getMethod() - .isAnnotationPresent(Transactional.class); + .isAnnotationPresent( + Transactional.class); // create a task which will be scheduled with executor final Runnable toSchedule = createRunnableToSchedule(throttledFuture, identifier, withTransaction); @@ -391,14 +394,15 @@ private Runnable createRunnableToSchedule(ThrottledFuture throttledFuture, Id final Long threadId = Thread.currentThread().getId(); throttledThreads.add(threadId); - LOG.trace("Running throttled task [{} left] [{} running] '{}'", countRemaining() - 1, countRunning(), identifier); + LOG.trace("Running throttled task [{} left] [{} running] '{}'", countRemaining() - 1, countRunning(), + identifier); // restore the security context SecurityContextHolder.setContext(securityContext.get()); try { // fulfill the future if (withTransaction) { - transactionExecutor.execute(()->throttledFuture.run(this::notifyTaskChanged)); + transactionExecutor.execute(() -> throttledFuture.run(this::notifyTaskChanged)); } else { throttledFuture.run(this::notifyTaskChanged); } @@ -413,7 +417,8 @@ private Runnable createRunnableToSchedule(ThrottledFuture throttledFuture, Id notifyTaskChanged(throttledFuture); // task done // clear the security context SecurityContextHolder.clearContext(); - LOG.trace("Finished throttled task [{} left] [{} running] '{}'", countRemaining(), countRunning() - 1, identifier); + LOG.trace("Finished throttled task [{} left] [{} running] '{}'", countRemaining(), countRunning() - 1, + identifier); clearOldFutures(); @@ -425,13 +430,16 @@ private Runnable createRunnableToSchedule(ThrottledFuture throttledFuture, Id /** * Discards futures from {@link #throttledFutures}, {@link #lastRun} and {@link #scheduledFutures} maps. - *

Every completed future for which a {@link Configuration#throttleDiscardThreshold throttleDiscardThreshold} expired is discarded.

+ *

+ * Every completed future for which a {@link Configuration#getThrottleDiscardThreshold()} expired is discarded. + * * @see #isThresholdExpired(Identifier) */ private void clearOldFutures() { // if the last clear was performed less than a threshold ago, skip it for now Instant last = lastClear.get(); - if (last.isAfter(Instant.now(clock).minus(configuration.getThrottleThreshold()).minus(configuration.getThrottleDiscardThreshold()))) { + if (last.isAfter(Instant.now(clock).minus(configuration.getThrottleThreshold()) + .minus(configuration.getThrottleDiscardThreshold()))) { return; } if (!lastClear.compareAndSet(last, Instant.now(clock))) { @@ -444,7 +452,8 @@ private void clearOldFutures() { .stream()) .flatMap(s -> s).distinct().toList() // ensures safe modification of maps .forEach(identifier -> { - if (isThresholdExpiredByMoreThan(identifier, configuration.getThrottleDiscardThreshold())) { + if (isThresholdExpiredByMoreThan(identifier, + configuration.getThrottleDiscardThreshold())) { Optional.ofNullable(throttledFutures.get(identifier)).ifPresent(throttled -> { if (throttled.isDone()) { throttledFutures.remove(identifier); @@ -465,20 +474,22 @@ private void clearOldFutures() { /** * @param identifier of the task - * @param duration to add to the throttle threshold - * @return Whether the last time when a task with specified {@code identifier} run - * is older than ({@link Configuration#throttleThreshold throttleThreshold} + {@code duration}) + * @param duration to add to the throttle threshold + * @return Whether the last time when a task with specified {@code identifier} run is older than + * ({@link Configuration#getThrottleThreshold()} + {@code duration}) */ private boolean isThresholdExpiredByMoreThan(Identifier identifier, Duration duration) { - return lastRun.getOrDefault(identifier, Instant.MAX).isBefore(Instant.now(clock).minus(configuration.getThrottleThreshold()).minus(duration)); + return lastRun.getOrDefault(identifier, Instant.MAX) + .isBefore(Instant.now(clock).minus(configuration.getThrottleThreshold()).minus(duration)); } /** - * @return Whether the time when the identifier last run is older than the threshold, - * true when the task had never run + * @return Whether the time when the identifier last run is older than the threshold, true when the task had never + * run */ private boolean isThresholdExpired(Identifier identifier) { - return lastRun.getOrDefault(identifier, Instant.EPOCH).isBefore(Instant.now(clock).minus(configuration.getThrottleThreshold())); + return lastRun.getOrDefault(identifier, Instant.EPOCH) + .isBefore(Instant.now(clock).minus(configuration.getThrottleThreshold())); } @SuppressWarnings("unchecked") @@ -544,7 +555,8 @@ private Identifier makeIdentifier(JoinPoint joinPoint, Throttle throttleAnnotati if (Void.TYPE.equals(returnType) || Void.class.equals(returnType)) { return null; } - throw new ThrottleAspectException("Invalid return type for " + signature + " annotated with @Debounce, only Future or void allowed!"); + throw new ThrottleAspectException( + "Invalid return type for " + signature + " annotated with @Debounce, only Future or void allowed!"); } @@ -573,7 +585,8 @@ private Identifier makeIdentifier(JoinPoint joinPoint, Throttle throttleAnnotati Objects.requireNonNull(identifierList); return identifierList.stream().map(Object::toString).collect(Collectors.joining("-")); } catch (EvaluationException | ClassCastException | NullPointerException e) { - throw new ThrottleAspectException("The expression: '" + expression + "' has not been resolved to a Collection or String", e); + throw new ThrottleAspectException( + "The expression: '" + expression + "' has not been resolved to a Collection or String", e); } }