diff --git a/services/optimizer/src/main/java/com/linkedin/openhouse/optimizer/OptimizerServiceApplication.java b/services/optimizer/src/main/java/com/linkedin/openhouse/optimizer/OptimizerServiceApplication.java index 38eb363a8..5a8cd4672 100644 --- a/services/optimizer/src/main/java/com/linkedin/openhouse/optimizer/OptimizerServiceApplication.java +++ b/services/optimizer/src/main/java/com/linkedin/openhouse/optimizer/OptimizerServiceApplication.java @@ -1,10 +1,13 @@ package com.linkedin.openhouse.optimizer; +import com.linkedin.openhouse.optimizer.api.spec.ApiListLimitProperties; import org.springframework.boot.SpringApplication; import org.springframework.boot.autoconfigure.SpringBootApplication; +import org.springframework.boot.context.properties.EnableConfigurationProperties; /** Spring Boot entry point for the Optimizer Service. */ @SpringBootApplication +@EnableConfigurationProperties(ApiListLimitProperties.class) public class OptimizerServiceApplication { public static void main(String[] args) { diff --git a/services/optimizer/src/main/java/com/linkedin/openhouse/optimizer/api/controller/TableOperationsController.java b/services/optimizer/src/main/java/com/linkedin/openhouse/optimizer/api/controller/TableOperationsController.java index c28002bf7..13464ab94 100644 --- a/services/optimizer/src/main/java/com/linkedin/openhouse/optimizer/api/controller/TableOperationsController.java +++ b/services/optimizer/src/main/java/com/linkedin/openhouse/optimizer/api/controller/TableOperationsController.java @@ -1,5 +1,6 @@ package com.linkedin.openhouse.optimizer.api.controller; +import com.linkedin.openhouse.optimizer.api.spec.ApiListLimitProperties; import com.linkedin.openhouse.optimizer.api.spec.OperationStatus; import com.linkedin.openhouse.optimizer.api.spec.OperationType; import com.linkedin.openhouse.optimizer.api.spec.TableOperations; @@ -27,6 +28,7 @@ public class TableOperationsController { private final OptimizerDataService service; + private final ApiListLimitProperties limits; /** * Report an update to an operation. The body carries the {@code operationId} the caller is @@ -59,8 +61,9 @@ public ResponseEntity getTableOperation(@PathVariable String id } /** - * List operations matching the given filters. All parameters are optional — omit all to return - * every row. + * List operations matching the given filters. Every filter is optional. {@code limit} defaults to + * {@code optimizer.api.list.default-limit} (10) and is rejected with 400 outside {@code [1, + * optimizer.api.list.max-limit]} (1000). */ @GetMapping public ResponseEntity> listTableOperations( @@ -68,7 +71,9 @@ public ResponseEntity> listTableOperations( @RequestParam(required = false) OperationStatus status, @RequestParam(required = false) String databaseName, @RequestParam(required = false) String tableName, - @RequestParam(required = false) String tableUuid) { + @RequestParam(required = false) String tableUuid, + @RequestParam(required = false) Integer limit) { + int effectiveLimit = limits.validateAndResolve(limit); List result = service .listTableOperations( @@ -76,7 +81,8 @@ public ResponseEntity> listTableOperations( Optional.ofNullable(status).map(OperationStatus::toModel), Optional.ofNullable(databaseName), Optional.ofNullable(tableName), - Optional.ofNullable(tableUuid)) + Optional.ofNullable(tableUuid), + effectiveLimit) .stream() .map(TableOperations::fromModel) .collect(Collectors.toList()); diff --git a/services/optimizer/src/main/java/com/linkedin/openhouse/optimizer/api/controller/TableOperationsHistoryController.java b/services/optimizer/src/main/java/com/linkedin/openhouse/optimizer/api/controller/TableOperationsHistoryController.java index 36c422623..5b01e3d8b 100644 --- a/services/optimizer/src/main/java/com/linkedin/openhouse/optimizer/api/controller/TableOperationsHistoryController.java +++ b/services/optimizer/src/main/java/com/linkedin/openhouse/optimizer/api/controller/TableOperationsHistoryController.java @@ -1,5 +1,6 @@ package com.linkedin.openhouse.optimizer.api.controller; +import com.linkedin.openhouse.optimizer.api.spec.ApiListLimitProperties; import com.linkedin.openhouse.optimizer.api.spec.TableOperationsHistory; import com.linkedin.openhouse.optimizer.service.OptimizerDataService; import java.util.List; @@ -22,6 +23,7 @@ public class TableOperationsHistoryController { private final OptimizerDataService service; + private final ApiListLimitProperties limits; /** Append a completed-job result. Called by the SparkJob after each run (success or failure). */ @PostMapping @@ -31,12 +33,17 @@ public ResponseEntity appendHistory( .body(TableOperationsHistory.fromModel(service.appendHistory(dto.toModel()))); } - /** Return the most recent history for a table, newest first, up to {@code limit} rows. */ + /** + * Return the most recent history for a table, newest first. {@code limit} defaults to {@code + * optimizer.api.list.default-limit} (10) and is rejected with 400 outside {@code [1, + * optimizer.api.list.max-limit]} (1000). + */ @GetMapping("/{tableUuid}") public ResponseEntity> getHistory( - @PathVariable String tableUuid, @RequestParam(defaultValue = "100") int limit) { + @PathVariable String tableUuid, @RequestParam(required = false) Integer limit) { + int effectiveLimit = limits.validateAndResolve(limit); List result = - service.getHistory(tableUuid, limit).stream() + service.getHistory(tableUuid, effectiveLimit).stream() .map(TableOperationsHistory::fromModel) .collect(Collectors.toList()); return ResponseEntity.ok(result); diff --git a/services/optimizer/src/main/java/com/linkedin/openhouse/optimizer/api/controller/TableStatsController.java b/services/optimizer/src/main/java/com/linkedin/openhouse/optimizer/api/controller/TableStatsController.java index 7cb745250..59b82919e 100644 --- a/services/optimizer/src/main/java/com/linkedin/openhouse/optimizer/api/controller/TableStatsController.java +++ b/services/optimizer/src/main/java/com/linkedin/openhouse/optimizer/api/controller/TableStatsController.java @@ -1,5 +1,6 @@ package com.linkedin.openhouse.optimizer.api.controller; +import com.linkedin.openhouse.optimizer.api.spec.ApiListLimitProperties; import com.linkedin.openhouse.optimizer.api.spec.TableStats; import com.linkedin.openhouse.optimizer.api.spec.TableStatsHistory; import com.linkedin.openhouse.optimizer.api.spec.UpsertTableStatsRequest; @@ -25,6 +26,7 @@ public class TableStatsController { private final OptimizerDataService service; + private final ApiListLimitProperties limits; /** * Create or overwrite the stats row for {@code tableUuid}. Called by the Tables Service on every @@ -48,20 +50,24 @@ public ResponseEntity getTableStats(@PathVariable String tableUuid) } /** - * List stats rows matching the given filters. All parameters are optional — omit all to return - * every row. + * List stats rows matching the given filters. Every filter is optional. {@code limit} defaults to + * {@code optimizer.api.list.default-limit} (10) and is rejected with 400 outside {@code [1, + * optimizer.api.list.max-limit]} (1000). */ @GetMapping public ResponseEntity> listTableStats( @RequestParam(required = false) String databaseName, @RequestParam(required = false) String tableName, - @RequestParam(required = false) String tableUuid) { + @RequestParam(required = false) String tableUuid, + @RequestParam(required = false) Integer limit) { + int effectiveLimit = limits.validateAndResolve(limit); List result = service .listTableStats( Optional.ofNullable(databaseName), Optional.ofNullable(tableName), - Optional.ofNullable(tableUuid)) + Optional.ofNullable(tableUuid), + effectiveLimit) .stream() .map(TableStats::fromModel) .collect(Collectors.toList()); @@ -69,16 +75,18 @@ public ResponseEntity> listTableStats( } /** - * Return per-commit stats history for {@code tableUuid}, newest first. Optionally filter by - * {@code since} (inclusive) and cap at {@code limit} rows. + * Return per-commit stats history for {@code tableUuid}, newest first. Optional {@code since} + * filter (inclusive). {@code limit} defaults to {@code optimizer.api.list.default-limit} (10) and + * is rejected with 400 outside {@code [1, optimizer.api.list.max-limit]} (1000). */ @GetMapping("/{tableUuid}/history") public ResponseEntity> getStatsHistory( @PathVariable String tableUuid, @RequestParam(required = false) Instant since, - @RequestParam(defaultValue = "100") int limit) { + @RequestParam(required = false) Integer limit) { + int effectiveLimit = limits.validateAndResolve(limit); List result = - service.getStatsHistory(tableUuid, Optional.ofNullable(since), limit).stream() + service.getStatsHistory(tableUuid, Optional.ofNullable(since), effectiveLimit).stream() .map(TableStatsHistory::fromModel) .collect(Collectors.toList()); return ResponseEntity.ok(result); diff --git a/services/optimizer/src/main/java/com/linkedin/openhouse/optimizer/api/spec/ApiListLimitProperties.java b/services/optimizer/src/main/java/com/linkedin/openhouse/optimizer/api/spec/ApiListLimitProperties.java new file mode 100644 index 000000000..0ffd3a9e3 --- /dev/null +++ b/services/optimizer/src/main/java/com/linkedin/openhouse/optimizer/api/spec/ApiListLimitProperties.java @@ -0,0 +1,34 @@ +package com.linkedin.openhouse.optimizer.api.spec; + +import lombok.Getter; +import lombok.Setter; +import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.http.HttpStatus; +import org.springframework.web.server.ResponseStatusException; + +/** + * Bounds on list-style endpoints. Default applies when the caller omits {@code limit}; max is the + * hard cap — requests beyond it are rejected (400) rather than silently clamped, so callers don't + * get partial results without knowing. + */ +@ConfigurationProperties("optimizer.api.list") +@Getter +@Setter +public class ApiListLimitProperties { + private int defaultLimit = 10; + private int maxLimit = 1000; + + /** + * Resolve a caller-supplied {@code limit} against this configuration. Null falls back to {@link + * #defaultLimit}; out-of-range values throw {@link ResponseStatusException} carrying HTTP 400. + */ + public int validateAndResolve(Integer limit) { + int effective = (limit == null) ? defaultLimit : limit; + if (effective < 1 || effective > maxLimit) { + throw new ResponseStatusException( + HttpStatus.BAD_REQUEST, + "limit must be between 1 and " + maxLimit + " (got " + effective + ")"); + } + return effective; + } +} diff --git a/services/optimizer/src/main/java/com/linkedin/openhouse/optimizer/service/OptimizerDataService.java b/services/optimizer/src/main/java/com/linkedin/openhouse/optimizer/service/OptimizerDataService.java index 0529d3608..c20ae7bf2 100644 --- a/services/optimizer/src/main/java/com/linkedin/openhouse/optimizer/service/OptimizerDataService.java +++ b/services/optimizer/src/main/java/com/linkedin/openhouse/optimizer/service/OptimizerDataService.java @@ -23,15 +23,16 @@ public interface OptimizerDataService { // --- TableOperations --- /** - * List operations matching the given filters. Every parameter is optional — pass {@link - * Optional#empty()} to skip that filter. No filters returns all rows. + * List operations matching the given filters, capped at {@code limit} rows. Every filter + * parameter is optional — pass {@link Optional#empty()} to skip that filter. */ List listTableOperations( Optional operationType, Optional status, Optional databaseName, Optional tableName, - Optional tableUuid); + Optional tableUuid, + int limit); /** * Update an operation by writing a history entry. Looks up the operation row by {@code @@ -60,11 +61,14 @@ List listTableOperations( Optional getTableStats(String tableUuid); /** - * List stats rows matching the given filters. Every parameter is optional — pass {@link - * Optional#empty()} to skip that filter. No filters returns all rows. + * List stats rows matching the given filters, capped at {@code limit} rows. Every filter + * parameter is optional — pass {@link Optional#empty()} to skip that filter. */ List listTableStats( - Optional databaseName, Optional tableName, Optional tableUuid); + Optional databaseName, + Optional tableName, + Optional tableUuid, + int limit); /** * Return per-commit stats history for {@code tableUuid}, newest first. diff --git a/services/optimizer/src/main/java/com/linkedin/openhouse/optimizer/service/OptimizerDataServiceImpl.java b/services/optimizer/src/main/java/com/linkedin/openhouse/optimizer/service/OptimizerDataServiceImpl.java index 4f820e1b8..29fd0eeee 100644 --- a/services/optimizer/src/main/java/com/linkedin/openhouse/optimizer/service/OptimizerDataServiceImpl.java +++ b/services/optimizer/src/main/java/com/linkedin/openhouse/optimizer/service/OptimizerDataServiceImpl.java @@ -19,7 +19,6 @@ import java.util.UUID; import java.util.stream.Collectors; import lombok.RequiredArgsConstructor; -import org.springframework.beans.factory.annotation.Value; import org.springframework.data.domain.PageRequest; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; @@ -40,9 +39,6 @@ public class OptimizerDataServiceImpl implements OptimizerDataService { private final TableStatsRepository statsRepository; private final TableStatsHistoryRepository statsHistoryRepository; - @Value("${optimizer.repo.default-limit:10000}") - private int defaultLimit; - // --- TableOperations --- @Override @@ -51,7 +47,8 @@ public List listTableOperations( Optional status, Optional databaseName, Optional tableName, - Optional tableUuid) { + Optional tableUuid, + int limit) { return operationsRepository .find( operationType.map(OperationTypeDto::toDb), @@ -61,7 +58,7 @@ public List listTableOperations( tableName, Optional.empty(), Optional.empty(), - PageRequest.of(0, defaultLimit)) + PageRequest.of(0, limit)) .stream() .map(TableOperationDto::fromRow) .collect(Collectors.toList()); @@ -137,8 +134,11 @@ public Optional getTableStats(String tableUuid) { @Override public List listTableStats( - Optional databaseName, Optional tableName, Optional tableUuid) { - return statsRepository.find(databaseName, tableName, tableUuid, PageRequest.of(0, defaultLimit)) + Optional databaseName, + Optional tableName, + Optional tableUuid, + int limit) { + return statsRepository.find(databaseName, tableName, tableUuid, PageRequest.of(0, limit)) .stream() .map(TableStatsDto::fromRow) .collect(Collectors.toList()); diff --git a/services/optimizer/src/main/resources/application.properties b/services/optimizer/src/main/resources/application.properties index 1b7eb1a40..2cb4dabeb 100644 --- a/services/optimizer/src/main/resources/application.properties +++ b/services/optimizer/src/main/resources/application.properties @@ -16,7 +16,8 @@ spring.datasource.username=${OPTIMIZER_DB_USER:oh_user} spring.datasource.password=${OPTIMIZER_DB_PASSWORD:oh_password} spring.datasource.hikari.maximum-pool-size=20 -optimizer.repo.default-limit=10000 +optimizer.api.list.default-limit=10 +optimizer.api.list.max-limit=1000 management.endpoints.web.exposure.include=health,prometheus management.endpoint.health.enabled=true diff --git a/services/optimizer/src/test/java/com/linkedin/openhouse/optimizer/api/controller/ListLimitControllerTest.java b/services/optimizer/src/test/java/com/linkedin/openhouse/optimizer/api/controller/ListLimitControllerTest.java new file mode 100644 index 000000000..2b00c5b1e --- /dev/null +++ b/services/optimizer/src/test/java/com/linkedin/openhouse/optimizer/api/controller/ListLimitControllerTest.java @@ -0,0 +1,176 @@ +package com.linkedin.openhouse.optimizer.api.controller; + +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import com.linkedin.openhouse.optimizer.db.OperationStatus; +import com.linkedin.openhouse.optimizer.db.OperationType; +import com.linkedin.openhouse.optimizer.db.TableOperationsHistoryRow; +import com.linkedin.openhouse.optimizer.db.TableOperationsRow; +import com.linkedin.openhouse.optimizer.db.TableStatsHistoryRow; +import com.linkedin.openhouse.optimizer.db.TableStatsRow; +import com.linkedin.openhouse.optimizer.repository.TableOperationsHistoryRepository; +import com.linkedin.openhouse.optimizer.repository.TableOperationsRepository; +import com.linkedin.openhouse.optimizer.repository.TableStatsHistoryRepository; +import com.linkedin.openhouse.optimizer.repository.TableStatsRepository; +import java.time.Instant; +import java.util.UUID; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.context.TestPropertySource; +import org.springframework.test.web.servlet.MockMvc; +import org.springframework.transaction.annotation.Transactional; + +/** + * Exercises the limit param across all four list endpoints: default applies when omitted, + * out-of-range values are rejected with 400, and the {@code max-limit} bound is config-driven. + */ +@SpringBootTest +@AutoConfigureMockMvc +@ActiveProfiles("test") +@TestPropertySource( + properties = { + "optimizer.api.list.default-limit=3", + "optimizer.api.list.max-limit=50", + }) +@Transactional +class ListLimitControllerTest { + + @Autowired MockMvc mockMvc; + @Autowired TableOperationsRepository operationsRepository; + @Autowired TableOperationsHistoryRepository historyRepository; + @Autowired TableStatsRepository statsRepository; + @Autowired TableStatsHistoryRepository statsHistoryRepository; + + private String sharedTableUuid; + + @BeforeEach + void seed() { + sharedTableUuid = UUID.randomUUID().toString(); + for (int i = 0; i < 8; i++) { + operationsRepository.save( + TableOperationsRow.builder() + .id(UUID.randomUUID().toString()) + .tableUuid(UUID.randomUUID().toString()) + .databaseName("db1") + .tableName("tbl" + i) + .operationType(OperationType.ORPHAN_FILES_DELETION) + .status(OperationStatus.PENDING) + .createdAt(Instant.now()) + .build()); + statsRepository.save( + TableStatsRow.builder() + .tableUuid(UUID.randomUUID().toString()) + .databaseName("db1") + .tableName("tbl" + i) + .updatedAt(Instant.now()) + .build()); + historyRepository.save( + TableOperationsHistoryRow.builder() + .id(UUID.randomUUID().toString()) + .tableUuid(sharedTableUuid) + .databaseName("db1") + .tableName("tbl") + .operationType(OperationType.ORPHAN_FILES_DELETION) + .status(com.linkedin.openhouse.optimizer.db.HistoryStatus.SUCCESS) + .completedAt(Instant.now()) + .build()); + statsHistoryRepository.save( + TableStatsHistoryRow.builder() + .id(UUID.randomUUID().toString()) + .tableUuid(sharedTableUuid) + .databaseName("db1") + .tableName("tbl") + .recordedAt(Instant.now()) + .build()); + } + } + + // --- /operations --- + + @Test + void listOperations_default_appliesConfiguredDefaultLimit() throws Exception { + mockMvc + .perform(get("/v1/optimizer/operations")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.length()").value(3)); + } + + @Test + void listOperations_explicitLimit_honored() throws Exception { + mockMvc + .perform(get("/v1/optimizer/operations").param("limit", "5")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.length()").value(5)); + } + + @Test + void listOperations_zeroLimit_returns400() throws Exception { + mockMvc + .perform(get("/v1/optimizer/operations").param("limit", "0")) + .andExpect(status().isBadRequest()); + } + + @Test + void listOperations_overMax_returns400() throws Exception { + mockMvc + .perform(get("/v1/optimizer/operations").param("limit", "51")) + .andExpect(status().isBadRequest()); + } + + // --- /stats --- + + @Test + void listStats_default_appliesConfiguredDefaultLimit() throws Exception { + mockMvc + .perform(get("/v1/optimizer/stats")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.length()").value(3)); + } + + @Test + void listStats_overMax_returns400() throws Exception { + mockMvc + .perform(get("/v1/optimizer/stats").param("limit", "51")) + .andExpect(status().isBadRequest()); + } + + // --- /operations-history/{tableUuid} --- + + @Test + void getOperationsHistory_default_appliesConfiguredDefaultLimit() throws Exception { + mockMvc + .perform(get("/v1/optimizer/operations-history/" + sharedTableUuid)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.length()").value(3)); + } + + @Test + void getOperationsHistory_overMax_returns400() throws Exception { + mockMvc + .perform(get("/v1/optimizer/operations-history/" + sharedTableUuid).param("limit", "51")) + .andExpect(status().isBadRequest()); + } + + // --- /stats/{tableUuid}/history --- + + @Test + void getStatsHistory_default_appliesConfiguredDefaultLimit() throws Exception { + mockMvc + .perform(get("/v1/optimizer/stats/" + sharedTableUuid + "/history")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.length()").value(3)); + } + + @Test + void getStatsHistory_overMax_returns400() throws Exception { + mockMvc + .perform(get("/v1/optimizer/stats/" + sharedTableUuid + "/history").param("limit", "51")) + .andExpect(status().isBadRequest()); + } +} diff --git a/services/optimizer/src/test/java/com/linkedin/openhouse/optimizer/service/OptimizerDataServiceImplTest.java b/services/optimizer/src/test/java/com/linkedin/openhouse/optimizer/service/OptimizerDataServiceImplTest.java index 8db14c4d6..437439eaf 100644 --- a/services/optimizer/src/test/java/com/linkedin/openhouse/optimizer/service/OptimizerDataServiceImplTest.java +++ b/services/optimizer/src/test/java/com/linkedin/openhouse/optimizer/service/OptimizerDataServiceImplTest.java @@ -165,8 +165,52 @@ void listTableOperations_filtersByOperationTypeAndStatus() { Optional.of(OperationStatusDto.PENDING), Optional.empty(), Optional.empty(), - Optional.empty())) + Optional.empty(), + 100)) .extracting(op -> op.getId()) .containsExactly(pendingId); } + + @Test + void listTableOperations_respectsLimit() { + for (int i = 0; i < 15; i++) { + operationsRepository.save( + TableOperationsRow.builder() + .id(UUID.randomUUID().toString()) + .tableUuid(UUID.randomUUID().toString()) + .databaseName("db1") + .tableName("tbl" + i) + .operationType( + com.linkedin.openhouse.optimizer.db.OperationType.ORPHAN_FILES_DELETION) + .status(com.linkedin.openhouse.optimizer.db.OperationStatus.PENDING) + .createdAt(Instant.now()) + .build()); + } + + assertThat( + service.listTableOperations( + Optional.empty(), + Optional.empty(), + Optional.empty(), + Optional.empty(), + Optional.empty(), + 5)) + .hasSize(5); + } + + @Test + void listTableStats_respectsLimit() { + for (int i = 0; i < 15; i++) { + service.upsertTableStats( + TableStatsDto.builder() + .tableUuid(UUID.randomUUID().toString()) + .databaseName("db1") + .tableName("tbl" + i) + .snapshot(TableStatsDto.SnapshotMetrics.builder().tableSizeBytes(1L).build()) + .build()); + } + + assertThat(service.listTableStats(Optional.empty(), Optional.empty(), Optional.empty(), 7)) + .hasSize(7); + } }