(wip) feat(optimizer): basic error-code handling across controllers

services/optimizer/src/main/java/com/linkedin/openhouse/optimizer/api/controller/TableOperationsController.java

@@ -7,18 +7,21 @@
 import com.linkedin.openhouse.optimizer.api.spec.UpdateOperationRequest;
 import com.linkedin.openhouse.optimizer.service.OptimizerDataService;
 import java.util.List;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.stream.Collectors;
 import lombok.RequiredArgsConstructor;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.ResponseEntity;
+import org.springframework.util.StringUtils;
 import org.springframework.web.bind.annotation.GetMapping;
 import org.springframework.web.bind.annotation.PathVariable;
 import org.springframework.web.bind.annotation.PostMapping;
 import org.springframework.web.bind.annotation.RequestBody;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RequestParam;
 import org.springframework.web.bind.annotation.RestController;
+import org.springframework.web.server.ResponseStatusException;
 
 /** REST controller for {@code table_operations}. */
 @RestController
@@ -29,23 +32,37 @@ public class TableOperationsController {
   private final OptimizerDataService service;
 
   /**
-   * Report an update to an operation. The body carries the {@code operationId} the caller is
-   * updating along with its terminal status. The backend looks up the operation row, writes a
-   * history entry with the operation's table metadata, and returns 201 Created with the history
-   * row, or 404 if the operation does not exist.
+   * Report an update to an operation. {@code id} comes from the URL; the body's {@code operationId}
+   * must match (the controller rejects mismatched requests with 400). The backend looks up the
+   * operation row, writes a history entry with the operation's table metadata, and returns 201
+   * Created with the history row, or 404 if the operation does not exist.
    */
-  @PostMapping("/update")
+  @PostMapping("/{id}/update")
   public ResponseEntity<TableOperationsHistory> updateOperation(
-      @RequestBody UpdateOperationRequest request) {
+      @PathVariable String id, @RequestBody UpdateOperationRequest request) {
+    if (!StringUtils.hasText(request.getOperationId())) {
+      throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "operationId is required");
+    }
+    if (!Objects.equals(id, request.getOperationId())) {
+      throw new ResponseStatusException(
+          HttpStatus.BAD_REQUEST,
+          String.format(
+              "operationId in body (%s) does not match path id (%s)",
+              request.getOperationId(), id));
+    }
+    if (request.getStatus() == null) {
+      throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "status is required");
+    }
     return service
-        .updateOperation(
-            request.getOperationId(),
-            request.getStatus() == null ? null : request.getStatus().toModel())
+        .updateOperation(id, request.getStatus().toModel())
         .map(
             history ->
                 ResponseEntity.status(HttpStatus.CREATED)
                     .body(TableOperationsHistory.fromModel(history)))
-        .orElse(ResponseEntity.notFound().build());
+        .orElseThrow(
+            () ->
+                new ResponseStatusException(
+                    HttpStatus.NOT_FOUND, String.format("no operation with id %s", id)));
   }
 
   /** Fetch a single operation row by its ID, regardless of status. Returns 404 if not found. */
@@ -55,28 +72,33 @@ public ResponseEntity<TableOperations> getTableOperation(@PathVariable String id
         .getTableOperation(id)
         .map(TableOperations::fromModel)
         .map(ResponseEntity::ok)
-        .orElse(ResponseEntity.notFound().build());
+        .orElseThrow(
+            () ->
+                new ResponseStatusException(
+                    HttpStatus.NOT_FOUND, String.format("no operation with id %s", id)));
   }
 
   /**
-   * List operations matching the given filters. All parameters are optional — omit all to return
-   * every row.
+   * List operations matching the given filters, capped at {@code limit} rows. Every filter is
+   * optional; {@code limit} is required so callers always state how much they want back.
    */
   @GetMapping
   public ResponseEntity<List<TableOperations>> listTableOperations(
       @RequestParam(required = false) OperationType operationType,
       @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 int limit) {
     List<TableOperations> result =
         service
             .listTableOperations(
                 Optional.ofNullable(operationType).map(OperationType::toModel),
                 Optional.ofNullable(status).map(OperationStatus::toModel),
                 Optional.ofNullable(databaseName),
                 Optional.ofNullable(tableName),
-                Optional.ofNullable(tableUuid))
+                Optional.ofNullable(tableUuid),
+                limit)
             .stream()
             .map(TableOperations::fromModel)
             .collect(Collectors.toList());

services/optimizer/src/main/java/com/linkedin/openhouse/optimizer/api/controller/TableOperationsHistoryController.java

@@ -31,10 +31,13 @@ public ResponseEntity<TableOperationsHistory> 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, capped at {@code limit} rows. {@code
+   * limit} is required.
+   */
   @GetMapping("/{tableUuid}")
   public ResponseEntity<List<TableOperationsHistory>> getHistory(
-      @PathVariable String tableUuid, @RequestParam(defaultValue = "100") int limit) {
+      @PathVariable String tableUuid, @RequestParam int limit) {
     List<TableOperationsHistory> result =
         service.getHistory(tableUuid, limit).stream()
             .map(TableOperationsHistory::fromModel)

services/optimizer/src/main/java/com/linkedin/openhouse/optimizer/api/controller/TableStatsController.java

@@ -9,6 +9,7 @@
 import java.util.Optional;
 import java.util.stream.Collectors;
 import lombok.RequiredArgsConstructor;
+import org.springframework.http.HttpStatus;
 import org.springframework.http.ResponseEntity;
 import org.springframework.web.bind.annotation.GetMapping;
 import org.springframework.web.bind.annotation.PathVariable;
@@ -17,6 +18,7 @@
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RequestParam;
 import org.springframework.web.bind.annotation.RestController;
+import org.springframework.web.server.ResponseStatusException;
 
 /** REST controller for managing per-table stats in the optimizer DB. */
 @RestController
@@ -44,39 +46,44 @@ public ResponseEntity<TableStats> getTableStats(@PathVariable String tableUuid)
         .getTableStats(tableUuid)
         .map(TableStats::fromModel)
         .map(ResponseEntity::ok)
-        .orElse(ResponseEntity.notFound().build());
+        .orElseThrow(
+            () ->
+                new ResponseStatusException(
+                    HttpStatus.NOT_FOUND, String.format("no stats for tableUuid %s", 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, capped at {@code limit} rows. Every filter is
+   * optional; {@code limit} is required so callers always state how much they want back.
    */
   @GetMapping
   public ResponseEntity<List<TableStats>> listTableStats(
       @RequestParam(required = false) String databaseName,
       @RequestParam(required = false) String tableName,
-      @RequestParam(required = false) String tableUuid) {
+      @RequestParam(required = false) String tableUuid,
+      @RequestParam int limit) {
     List<TableStats> result =
         service
             .listTableStats(
                 Optional.ofNullable(databaseName),
                 Optional.ofNullable(tableName),
-                Optional.ofNullable(tableUuid))
+                Optional.ofNullable(tableUuid),
+                limit)
             .stream()
             .map(TableStats::fromModel)
             .collect(Collectors.toList());
     return ResponseEntity.ok(result);
   }
 
   /**
-   * 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, capped at {@code limit}
+   * rows. Optional {@code since} filter (inclusive). {@code limit} is required.
    */
   @GetMapping("/{tableUuid}/history")
   public ResponseEntity<List<TableStatsHistory>> getStatsHistory(
       @PathVariable String tableUuid,
       @RequestParam(required = false) Instant since,
-      @RequestParam(defaultValue = "100") int limit) {
+      @RequestParam int limit) {
     List<TableStatsHistory> result =
         service.getStatsHistory(tableUuid, Optional.ofNullable(since), limit).stream()
             .map(TableStatsHistory::fromModel)

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<TableOperationDto> listTableOperations(
       Optional<OperationTypeDto> operationType,
       Optional<OperationStatusDto> status,
       Optional<String> databaseName,
       Optional<String> tableName,
-      Optional<String> tableUuid);
+      Optional<String> tableUuid,
+      int limit);
 
   /**
    * Update an operation by writing a history entry. Looks up the operation row by {@code
@@ -60,11 +61,14 @@ List<TableOperationDto> listTableOperations(
   Optional<TableStatsDto> 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<TableStatsDto> listTableStats(
-      Optional<String> databaseName, Optional<String> tableName, Optional<String> tableUuid);
+      Optional<String> databaseName,
+      Optional<String> tableName,
+      Optional<String> tableUuid,
+      int limit);
 
   /**
    * Return per-commit stats history for {@code tableUuid}, newest first.

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<TableOperationDto> listTableOperations(
       Optional<OperationStatusDto> status,
       Optional<String> databaseName,
       Optional<String> tableName,
-      Optional<String> tableUuid) {
+      Optional<String> tableUuid,
+      int limit) {
     return operationsRepository
         .find(
             operationType.map(OperationTypeDto::toDb),
@@ -61,7 +58,7 @@ public List<TableOperationDto> 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<TableStatsDto> getTableStats(String tableUuid) {
 
   @Override
   public List<TableStatsDto> listTableStats(
-      Optional<String> databaseName, Optional<String> tableName, Optional<String> tableUuid) {
-    return statsRepository.find(databaseName, tableName, tableUuid, PageRequest.of(0, defaultLimit))
+      Optional<String> databaseName,
+      Optional<String> tableName,
+      Optional<String> tableUuid,
+      int limit) {
+    return statsRepository.find(databaseName, tableName, tableUuid, PageRequest.of(0, limit))
         .stream()
         .map(TableStatsDto::fromRow)
         .collect(Collectors.toList());

services/optimizer/src/main/resources/application.properties

@@ -16,7 +16,10 @@ 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
-
 management.endpoints.web.exposure.include=health,prometheus
 management.endpoint.health.enabled=true
+
+# Include ResponseStatusException.reason in the default error response body. Without this, Spring
+# Boot 2.7 omits the `message` field, and the human-readable detail from a thrown
+# ResponseStatusException never reaches the caller.
+server.error.include-message=always