Skip to content

Commit

Permalink
fix(webserver): enforce pagination
Browse files Browse the repository at this point in the history
Added missing constraints at `page`/`size` query params.

Enforced pagination via `PageableUtils`.
  • Loading branch information
yuri1969 committed Sep 20, 2024
1 parent f494a9b commit daab1c8
Show file tree
Hide file tree
Showing 13 changed files with 127 additions and 40 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import io.swagger.v3.oas.annotations.Operation;
import io.swagger.v3.oas.annotations.Parameter;
import jakarta.inject.Inject;
import jakarta.validation.constraints.Min;
import lombok.*;
import lombok.experimental.FieldDefaults;
import lombok.experimental.SuperBuilder;
Expand All @@ -44,8 +45,8 @@ public PagedResults<BlueprintItem> blueprints(
@Parameter(description = "A string filter") @Nullable @QueryValue(value = "q") Optional<String> q,
@Parameter(description = "The sort of current page") @Nullable @QueryValue(value = "sort") Optional<String> sort,
@Parameter(description = "A tags filter") @Nullable @QueryValue(value = "tags") Optional<List<String>> tags,
@Parameter(description = "The current page") @QueryValue(defaultValue = "1") Integer page,
@Parameter(description = "The current page size") @QueryValue(defaultValue = "1") Integer size,
@Parameter(description = "The current page") @QueryValue(defaultValue = "1") @Min(1) Integer page,
@Parameter(description = "The current page size") @QueryValue(defaultValue = "1") @Min(1) Integer size,
HttpRequest<?> httpRequest
) throws URISyntaxException {
return fastForwardToKestraApi(httpRequest, "/v1/blueprints", Map.of("ee", false), Argument.of(PagedResults.class, BlueprintItem.class));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@
import io.micronaut.core.annotation.Nullable;
import io.micronaut.core.async.annotation.SingleResult;
import io.micronaut.core.convert.format.Format;
import io.micronaut.data.model.Pageable;
import io.micronaut.http.*;
import io.micronaut.http.annotation.*;
import io.micronaut.http.exceptions.HttpStatusException;
Expand All @@ -63,6 +62,7 @@
import jakarta.inject.Inject;
import jakarta.inject.Named;
import jakarta.validation.Valid;
import jakarta.validation.constraints.Min;
import jakarta.validation.constraints.NotNull;
import lombok.Getter;
import lombok.NoArgsConstructor;
Expand Down Expand Up @@ -149,8 +149,8 @@ public class ExecutionController {
@Get(uri = "/search")
@Operation(tags = {"Executions"}, summary = "Search for executions")
public PagedResults<Execution> find(
@Parameter(description = "The current page") @QueryValue(defaultValue = "1") int page,
@Parameter(description = "The current page size") @QueryValue(defaultValue = "10") int size,
@Parameter(description = "The current page") @QueryValue(defaultValue = "1") @Min(1) int page,
@Parameter(description = "The current page size") @QueryValue(defaultValue = "10") @Min(1) int size,
@Parameter(description = "The sort of current page") @Nullable @QueryValue List<String> sort,
@Parameter(description = "A string filter") @Nullable @QueryValue(value = "q") String query,
@Parameter(description = "The scope of the executions to include") @Nullable @QueryValue(value = "scope") List<FlowScope> scope,
Expand Down Expand Up @@ -401,12 +401,12 @@ public HttpResponse<BulkResponse> deleteByQuery(
public PagedResults<Execution> findByFlowId(
@Parameter(description = "The flow namespace") @QueryValue String namespace,
@Parameter(description = "The flow id") @QueryValue String flowId,
@Parameter(description = "The current page") @QueryValue(defaultValue = "1") int page,
@Parameter(description = "The current page size") @QueryValue(defaultValue = "10") int size
@Parameter(description = "The current page") @QueryValue(defaultValue = "1") @Min(1) int page,
@Parameter(description = "The current page size") @QueryValue(defaultValue = "10") @Min(1) int size
) {
return PagedResults.of(
executionRepository
.findByFlowId(tenantService.resolveTenant(), namespace, flowId, Pageable.from(page, size))
.findByFlowId(tenantService.resolveTenant(), namespace, flowId, PageableUtils.from(page, size))
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import io.swagger.v3.oas.annotations.media.Schema;
import io.swagger.v3.oas.annotations.responses.ApiResponse;
import jakarta.inject.Inject;
import jakarta.validation.constraints.Min;
import lombok.extern.slf4j.Slf4j;

import jakarta.validation.ConstraintViolationException;
Expand Down Expand Up @@ -195,8 +196,8 @@ public Object flowTask(
@Get(uri = "/search")
@Operation(tags = {"Flows"}, summary = "Search for flows")
public PagedResults<Flow> find(
@Parameter(description = "The current page") @QueryValue(defaultValue = "1") int page,
@Parameter(description = "The current page size") @QueryValue(defaultValue = "10") int size,
@Parameter(description = "The current page") @QueryValue(defaultValue = "1") @Min(1) int page,
@Parameter(description = "The current page size") @QueryValue(defaultValue = "10") @Min(1) int size,
@Parameter(description = "The sort of current page") @Nullable @QueryValue List<String> sort,
@Parameter(description = "A string filter") @Nullable @QueryValue(value = "q") String query,
@Parameter(description = "The scope of the flows to include") @Nullable @QueryValue List<FlowScope> scope,
Expand Down Expand Up @@ -227,8 +228,8 @@ public List<Flow> getFlowsByNamespace(
@Get(uri = "/source")
@Operation(tags = {"Flows"}, summary = "Search for flows source code")
public PagedResults<SearchResult<Flow>> source(
@Parameter(description = "The current page") @QueryValue(defaultValue = "1") int page,
@Parameter(description = "The current page size") @QueryValue(defaultValue = "10") int size,
@Parameter(description = "The current page") @QueryValue(defaultValue = "1") @Min(1) int page,
@Parameter(description = "The current page size") @QueryValue(defaultValue = "10") @Min(1) int size,
@Parameter(description = "The sort of current page") @Nullable @QueryValue List<String> sort,
@Parameter(description = "A string filter") @Nullable @QueryValue(value = "q") String query,
@Parameter(description = "A namespace filter prefix") @Nullable @QueryValue String namespace
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import io.swagger.v3.oas.annotations.Parameter;
import jakarta.inject.Inject;
import jakarta.inject.Named;
import jakarta.validation.constraints.Min;
import org.slf4j.event.Level;
import reactor.core.publisher.Flux;
import reactor.core.publisher.FluxSink;
Expand Down Expand Up @@ -53,8 +54,8 @@ public class LogController {
@Operation(tags = {"Logs"}, summary = "Search for logs")
public PagedResults<LogEntry> find(
@Parameter(description = "A string filter") @Nullable @QueryValue(value = "q") String query,
@Parameter(description = "The current page") @QueryValue(defaultValue = "1") int page,
@Parameter(description = "The current page size") @QueryValue(defaultValue = "10") int size,
@Parameter(description = "The current page") @QueryValue(defaultValue = "1") @Min(1) int page,
@Parameter(description = "The current page size") @QueryValue(defaultValue = "10") @Min(1) int size,
@Parameter(description = "The sort of current page") @Nullable @QueryValue List<String> sort,
@Parameter(description = "A namespace filter prefix") @Nullable @QueryValue String namespace,
@Parameter(description = "A flow id filter") @Nullable @QueryValue String flowId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import io.swagger.v3.oas.annotations.Parameter;
import jakarta.inject.Inject;
import jakarta.inject.Named;
import jakarta.validation.constraints.Min;

import java.time.ZonedDateTime;
import java.util.List;
Expand All @@ -46,8 +47,8 @@ public class MetricController {
@Get(uri = "/{executionId}")
@Operation(tags = {"Metrics"}, summary = "Get metrics for a specific execution")
public PagedResults<MetricEntry> findByExecution(
@Parameter(description = "The current page") @QueryValue(defaultValue = "1") int page,
@Parameter(description = "The current page size") @QueryValue(defaultValue = "10") int size,
@Parameter(description = "The current page") @QueryValue(defaultValue = "1") @Min(1) int page,
@Parameter(description = "The current page size") @QueryValue(defaultValue = "10") @Min(1) int size,
@Parameter(description = "The sort of current page") @Nullable @QueryValue List<String> sort,
@Parameter(description = "The execution id") @PathVariable String executionId,
@Parameter(description = "The taskrun id") @Nullable @QueryValue String taskRunId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import io.swagger.v3.oas.annotations.Operation;
import io.swagger.v3.oas.annotations.Parameter;
import jakarta.inject.Inject;
import jakarta.validation.constraints.Min;

import java.util.*;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -51,8 +52,8 @@ public Namespace index(
@Operation(tags = {"Namespaces"}, summary = "Search for namespaces")
public PagedResults<NamespaceWithDisabled> find(
@Parameter(description = "A string filter") @Nullable @QueryValue(value = "q") String query,
@Parameter(description = "The current page") @QueryValue(defaultValue = "1") int page,
@Parameter(description = "The current page size") @QueryValue(defaultValue = "10") int size,
@Parameter(description = "The current page") @QueryValue(defaultValue = "1") @Min(1) int page,
@Parameter(description = "The current page size") @QueryValue(defaultValue = "10") @Min(1) int size,
@Parameter(description = "The sort of current page") @Nullable @QueryValue List<String> sort,
@Parameter(description = "Return only existing namespace") @Nullable @QueryValue(value = "existing", defaultValue = "false") Boolean existingOnly
) throws HttpStatusException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import io.swagger.v3.oas.annotations.Operation;
import io.swagger.v3.oas.annotations.Parameter;
import jakarta.inject.Inject;
import jakarta.validation.constraints.Min;

import java.time.ZonedDateTime;
import java.util.List;
Expand All @@ -38,8 +39,8 @@ public class TaskRunController {
@Get(uri = "/search")
@Operation(tags = {"Executions"}, summary = "Search for taskruns")
public PagedResults<TaskRun> findTaskRun(
@Parameter(description = "The current page") @QueryValue(defaultValue = "1") int page,
@Parameter(description = "The current page size") @QueryValue(defaultValue = "10") int size,
@Parameter(description = "The current page") @QueryValue(defaultValue = "1") @Min(1) int page,
@Parameter(description = "The current page size") @QueryValue(defaultValue = "10") @Min(1) int size,
@Parameter(description = "The sort of current page") @Nullable @QueryValue List<String> sort,
@Parameter(description = "A string filter") @Nullable @QueryValue(value = "q") String query,
@Parameter(description = "A namespace filter prefix") @Nullable @QueryValue String namespace,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@

import jakarta.validation.ConstraintViolationException;
import jakarta.validation.Valid;
import jakarta.validation.constraints.Min;

import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.util.*;
Expand Down Expand Up @@ -68,8 +70,8 @@ public Template index(
@Get(uri = "/search")
@Operation(tags = {"Templates"}, summary = "Search for templates")
public PagedResults<Template> find(
@Parameter(description = "The current page") @QueryValue(defaultValue = "1") int page,
@Parameter(description = "The current page size") @QueryValue(defaultValue = "10") int size,
@Parameter(description = "The current page") @QueryValue(defaultValue = "1") @Min(1) int page,
@Parameter(description = "The current page size") @QueryValue(defaultValue = "10") @Min(1) int size,
@Parameter(description = "The sort of current page") @Nullable @QueryValue List<String> sort,
@Parameter(description = "A string filter") @Nullable @QueryValue(value = "q") String query,
@Parameter(description = "A namespace filter prefix") @Nullable @QueryValue String namespace
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import io.swagger.v3.oas.annotations.Operation;
import io.swagger.v3.oas.annotations.Parameter;
import jakarta.inject.Inject;
import jakarta.validation.constraints.Min;
import lombok.Builder;
import lombok.Getter;
import lombok.extern.slf4j.Slf4j;
Expand Down Expand Up @@ -69,8 +70,8 @@ public class TriggerController {
@Get(uri = "/search")
@Operation(tags = {"Triggers"}, summary = "Search for triggers")
public PagedResults<Triggers> search(
@Parameter(description = "The current page") @QueryValue(defaultValue = "1") int page,
@Parameter(description = "The current page size") @QueryValue(defaultValue = "10") int size,
@Parameter(description = "The current page") @QueryValue(defaultValue = "1") @Min(1) int page,
@Parameter(description = "The current page size") @QueryValue(defaultValue = "10") @Min(1) int size,
@Parameter(description = "The sort of current page") @Nullable @QueryValue List<String> sort,
@Parameter(description = "A string filter") @Nullable @QueryValue(value = "q") String query,
@Parameter(description = "A namespace filter prefix") @Nullable @QueryValue String namespace,
Expand Down Expand Up @@ -211,8 +212,8 @@ public MutableHttpResponse<?> unlockByQuery(
@Get(uri = "/{namespace}/{flowId}")
@Operation(tags = {"Triggers"}, summary = "Get all triggers for a flow")
public PagedResults<Trigger> find(
@Parameter(description = "The current page") @QueryValue(defaultValue = "1") int page,
@Parameter(description = "The current page size") @QueryValue(defaultValue = "10") int size,
@Parameter(description = "The current page") @QueryValue(defaultValue = "1") @Min(1) int page,
@Parameter(description = "The current page size") @QueryValue(defaultValue = "10") @Min(1) int size,
@Parameter(description = "The sort of current page") @Nullable @QueryValue List<String> sort,
@Parameter(description = "A string filter") @Nullable @QueryValue(value = "q") String query,
@Parameter(description = "A namespace filter prefix") @Nullable @QueryValue String namespace,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,23 +7,31 @@

import java.util.List;
import java.util.function.Function;
import java.util.stream.Collectors;

public class PageableUtils {
private PageableUtils() {
}

public static Pageable from(int page, int size, List<String> sort, Function<String, String> sortMapper) throws HttpStatusException {
return Pageable.from(
final Pageable pageable = Pageable.from(
page,
size,
sort(sort, sortMapper)
);

if (pageable.isUnpaged()) {
throw new IllegalArgumentException("Unpaged data are not supported");
}

return pageable;
}

public static Pageable from(int page, int size, List<String> sort) throws HttpStatusException {
return Pageable.from(
page,
size,
sort(sort, null)
);
return from(page, size, sort, null);
}

public static Pageable from(int page, int size) throws HttpStatusException {
return from(page, size, null, null);
}

protected static Sort sort(List<String> sort, Function<String, String> sortMapper) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1164,6 +1164,20 @@ void find() {
);

assertThat(e.getStatus(), is(HttpStatus.UNPROCESSABLE_ENTITY));

e = assertThrows(
HttpClientResponseException.class,
() -> client.toBlocking().retrieve(GET("/api/v1/executions/search?page=1&size=-1"))
);

assertThat(e.getStatus(), is(HttpStatus.UNPROCESSABLE_ENTITY));

e = assertThrows(
HttpClientResponseException.class,
() -> client.toBlocking().retrieve(GET("/api/v1/executions/search?page=0"))
);

assertThat(e.getStatus(), is(HttpStatus.UNPROCESSABLE_ENTITY));
}

// This test is flaky on CI as the flow may be already SUCCESS when we kill it if CI is super slow
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package io.kestra.webserver.controllers.api;

import io.kestra.core.models.executions.LogEntry;
import io.kestra.core.models.executions.MetricEntry;
import io.kestra.core.repositories.LogRepositoryInterface;
import io.kestra.core.utils.IdUtils;
import io.kestra.webserver.controllers.h2.JdbcH2ControllerTest;
Expand All @@ -11,6 +10,7 @@
import io.micronaut.http.HttpResponse;
import io.micronaut.http.HttpStatus;
import io.micronaut.http.client.annotation.Client;
import io.micronaut.http.client.exceptions.HttpClientResponseException;
import io.micronaut.reactor.http.client.ReactorHttpClient;
import io.micronaut.reactor.http.client.ReactorSseClient;
import jakarta.inject.Inject;
Expand All @@ -20,9 +20,11 @@
import java.time.Instant;
import java.util.List;

import static io.micronaut.http.HttpRequest.GET;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.is;
import static org.junit.jupiter.api.Assertions.assertThrows;

class LogControllerTest extends JdbcH2ControllerTest {

Expand All @@ -48,16 +50,30 @@ void find() {
logRepository.save(log3);

PagedResults<LogEntry> logs = client.toBlocking().retrieve(
HttpRequest.GET("/api/v1/logs/search"),
GET("/api/v1/logs/search"),
Argument.of(PagedResults.class, LogEntry.class)
);
assertThat(logs.getTotal(), is(3L));

logs = client.toBlocking().retrieve(
HttpRequest.GET("/api/v1/logs/search?minLevel=INFO"),
GET("/api/v1/logs/search?minLevel=INFO"),
Argument.of(PagedResults.class, LogEntry.class)
);
assertThat(logs.getTotal(), is(2L));

HttpClientResponseException e = assertThrows(
HttpClientResponseException.class,
() -> client.toBlocking().retrieve(GET("/api/v1/logs/search?page=1&size=-1"))
);

assertThat(e.getStatus(), is(HttpStatus.UNPROCESSABLE_ENTITY));

e = assertThrows(
HttpClientResponseException.class,
() -> client.toBlocking().retrieve(GET("/api/v1/logs/search?page=0"))
);

assertThat(e.getStatus(), is(HttpStatus.UNPROCESSABLE_ENTITY));
}

@SuppressWarnings("unchecked")
Expand All @@ -71,7 +87,7 @@ void findByExecution() {
logRepository.save(log3);

List<LogEntry> logs = client.toBlocking().retrieve(
HttpRequest.GET("/api/v1/logs/" + log1.getExecutionId()),
GET("/api/v1/logs/" + log1.getExecutionId()),
Argument.of(List.class, LogEntry.class)
);
assertThat(logs.size(), is(2));
Expand All @@ -89,7 +105,7 @@ void download() {
logRepository.save(log3);

String logs = client.toBlocking().retrieve(
HttpRequest.GET("/api/v1/logs/" + log1.getExecutionId() + "/download"),
GET("/api/v1/logs/" + log1.getExecutionId() + "/download"),
String.class
);
assertThat(logs, containsString("john doe"));
Expand All @@ -112,7 +128,7 @@ void delete() {
assertThat(delete.getStatus(), is(HttpStatus.OK));

List<LogEntry> logs = client.toBlocking().retrieve(
HttpRequest.GET("/api/v1/logs/" + log1.getExecutionId()),
GET("/api/v1/logs/" + log1.getExecutionId()),
Argument.of(List.class, LogEntry.class)
);
assertThat(logs.size(), is(0));
Expand All @@ -133,7 +149,7 @@ void deleteByQuery() {
assertThat(delete.getStatus(), is(HttpStatus.OK));

List<LogEntry> logs = client.toBlocking().retrieve(
HttpRequest.GET("/api/v1/logs/" + log1.getExecutionId()),
GET("/api/v1/logs/" + log1.getExecutionId()),
Argument.of(List.class, LogEntry.class)
);
assertThat(logs.size(), is(0));
Expand Down
Loading

0 comments on commit daab1c8

Please sign in to comment.