From 3427c1d4083d8b1345191d4ac0e15cc3773c0672 Mon Sep 17 00:00:00 2001 From: JinBa1 <72070041+JinBa1@users.noreply.github.com> Date: Sun, 14 Jun 2026 01:34:05 +0100 Subject: [PATCH 1/2] feat(server): add Spring Boot REST gateway over the query engine POST /queries plans and synchronously executes read-only SQL, returning a structured JSON result (positional column arrays + name/qualifiedName/type metadata, truncated/hint) or EXPLAIN plan text. The /tables endpoints list tables, expose a table's static typed schema, and accept opt-in CSV upload (disabled by default). A single QueryService choke point plans, attaches an always-present fail-closed budget (clamped to hard caps), bounds concurrency with a semaphore, branches EXPLAIN with zero budget, and audits every request, so governance cannot be bypassed. GlobalExceptionHandler maps engine ErrorCodes to HTTP: 4xx where the caller can act, scrubbed 5xx (generic message + id, paths logged server-side only) for engine faults. Strict table-name validation, work-dir path containment, and a streamed upload size cap guard the write surface. Spring enters only through the server module (spring-boot-dependencies BOM import, no reparent); the parent POM and engine stay Spring-free. JUnit/JMH version management moves to the engine module so the server takes JUnit from the Spring BOM. CI switches to `mvnw clean package` to exercise the boot jar. Engine: add BudgetKind{TUPLES,TIME} on QueryBudgetExceededException so a tuple-budget breach maps to 429 and a wall-clock breach to 504. --- .github/workflows/ci.yml | 8 +- engine/pom.xml | 4 + .../github/jinba1/cuckoodb/BudgetKind.java | 15 ++ .../github/jinba1/cuckoodb/QueryBudget.java | 6 +- .../QueryBudgetExceededException.java | 13 +- .../jinba1/cuckoodb/QueryBudgetTest.java | 26 +++ pom.xml | 26 +-- server/pom.xml | 67 +++++- .../server/CuckooDbServerApplication.java | 20 ++ .../cuckoodb/server/ServerPlaceholder.java | 28 --- .../cuckoodb/server/audit/AuditEvent.java | 42 ++++ .../cuckoodb/server/audit/AuditSink.java | 10 + .../cuckoodb/server/audit/NoOpAuditSink.java | 20 ++ .../server/catalog/CatalogFacade.java | 76 ++++++ .../server/config/CatalogInitializer.java | 57 +++++ .../server/config/CuckooDbProperties.java | 81 +++++++ .../cuckoodb/server/config/OpenApiConfig.java | 27 +++ .../server/query/ApiPrincipalResolver.java | 37 +++ .../cuckoodb/server/query/BudgetPolicy.java | 51 +++++ .../ConcurrencyLimitExceededException.java | 11 + .../server/query/ConcurrencyLimiter.java | 54 +++++ .../cuckoodb/server/query/QueryService.java | 84 +++++++ .../server/query/QueryServiceResult.java | 26 +++ .../server/web/GlobalExceptionHandler.java | 157 +++++++++++++ .../server/web/InvalidUploadException.java | 12 + .../server/web/PayloadTooLargeException.java | 11 + .../cuckoodb/server/web/QueryController.java | 42 ++++ .../web/TableAlreadyExistsException.java | 11 + .../cuckoodb/server/web/TableController.java | 181 +++++++++++++++ .../web/TableLimitExceededException.java | 12 + .../server/web/TableNotFoundException.java | 11 + .../server/web/UploadDisabledException.java | 11 + .../cuckoodb/server/web/dto/ColumnDto.java | 16 ++ .../server/web/dto/ErrorResponse.java | 26 +++ .../cuckoodb/server/web/dto/QueryRequest.java | 11 + .../server/web/dto/QueryResponse.java | 73 ++++++ .../server/web/dto/TableColumnDto.java | 10 + .../server/web/dto/TableSchemaResponse.java | 13 ++ .../server/web/dto/UploadResponse.java | 13 ++ .../src/main/resources/application.properties | 36 +++ .../server/query/BudgetPolicyTest.java | 48 ++++ .../server/query/ConcurrencyLimiterTest.java | 38 +++ .../server/web/BudgetIntegrationTest.java | 111 +++++++++ .../server/web/QueryApiIntegrationTest.java | 185 +++++++++++++++ .../server/web/QueryControllerTest.java | 216 ++++++++++++++++++ .../server/web/TableControllerTest.java | 87 +++++++ .../server/web/UploadApiIntegrationTest.java | 151 ++++++++++++ .../web/UploadLimitIntegrationTest.java | 92 ++++++++ 48 files changed, 2304 insertions(+), 59 deletions(-) create mode 100644 engine/src/main/java/com/github/jinba1/cuckoodb/BudgetKind.java create mode 100644 server/src/main/java/com/github/jinba1/cuckoodb/server/CuckooDbServerApplication.java delete mode 100644 server/src/main/java/com/github/jinba1/cuckoodb/server/ServerPlaceholder.java create mode 100644 server/src/main/java/com/github/jinba1/cuckoodb/server/audit/AuditEvent.java create mode 100644 server/src/main/java/com/github/jinba1/cuckoodb/server/audit/AuditSink.java create mode 100644 server/src/main/java/com/github/jinba1/cuckoodb/server/audit/NoOpAuditSink.java create mode 100644 server/src/main/java/com/github/jinba1/cuckoodb/server/catalog/CatalogFacade.java create mode 100644 server/src/main/java/com/github/jinba1/cuckoodb/server/config/CatalogInitializer.java create mode 100644 server/src/main/java/com/github/jinba1/cuckoodb/server/config/CuckooDbProperties.java create mode 100644 server/src/main/java/com/github/jinba1/cuckoodb/server/config/OpenApiConfig.java create mode 100644 server/src/main/java/com/github/jinba1/cuckoodb/server/query/ApiPrincipalResolver.java create mode 100644 server/src/main/java/com/github/jinba1/cuckoodb/server/query/BudgetPolicy.java create mode 100644 server/src/main/java/com/github/jinba1/cuckoodb/server/query/ConcurrencyLimitExceededException.java create mode 100644 server/src/main/java/com/github/jinba1/cuckoodb/server/query/ConcurrencyLimiter.java create mode 100644 server/src/main/java/com/github/jinba1/cuckoodb/server/query/QueryService.java create mode 100644 server/src/main/java/com/github/jinba1/cuckoodb/server/query/QueryServiceResult.java create mode 100644 server/src/main/java/com/github/jinba1/cuckoodb/server/web/GlobalExceptionHandler.java create mode 100644 server/src/main/java/com/github/jinba1/cuckoodb/server/web/InvalidUploadException.java create mode 100644 server/src/main/java/com/github/jinba1/cuckoodb/server/web/PayloadTooLargeException.java create mode 100644 server/src/main/java/com/github/jinba1/cuckoodb/server/web/QueryController.java create mode 100644 server/src/main/java/com/github/jinba1/cuckoodb/server/web/TableAlreadyExistsException.java create mode 100644 server/src/main/java/com/github/jinba1/cuckoodb/server/web/TableController.java create mode 100644 server/src/main/java/com/github/jinba1/cuckoodb/server/web/TableLimitExceededException.java create mode 100644 server/src/main/java/com/github/jinba1/cuckoodb/server/web/TableNotFoundException.java create mode 100644 server/src/main/java/com/github/jinba1/cuckoodb/server/web/UploadDisabledException.java create mode 100644 server/src/main/java/com/github/jinba1/cuckoodb/server/web/dto/ColumnDto.java create mode 100644 server/src/main/java/com/github/jinba1/cuckoodb/server/web/dto/ErrorResponse.java create mode 100644 server/src/main/java/com/github/jinba1/cuckoodb/server/web/dto/QueryRequest.java create mode 100644 server/src/main/java/com/github/jinba1/cuckoodb/server/web/dto/QueryResponse.java create mode 100644 server/src/main/java/com/github/jinba1/cuckoodb/server/web/dto/TableColumnDto.java create mode 100644 server/src/main/java/com/github/jinba1/cuckoodb/server/web/dto/TableSchemaResponse.java create mode 100644 server/src/main/java/com/github/jinba1/cuckoodb/server/web/dto/UploadResponse.java create mode 100644 server/src/main/resources/application.properties create mode 100644 server/src/test/java/com/github/jinba1/cuckoodb/server/query/BudgetPolicyTest.java create mode 100644 server/src/test/java/com/github/jinba1/cuckoodb/server/query/ConcurrencyLimiterTest.java create mode 100644 server/src/test/java/com/github/jinba1/cuckoodb/server/web/BudgetIntegrationTest.java create mode 100644 server/src/test/java/com/github/jinba1/cuckoodb/server/web/QueryApiIntegrationTest.java create mode 100644 server/src/test/java/com/github/jinba1/cuckoodb/server/web/QueryControllerTest.java create mode 100644 server/src/test/java/com/github/jinba1/cuckoodb/server/web/TableControllerTest.java create mode 100644 server/src/test/java/com/github/jinba1/cuckoodb/server/web/UploadApiIntegrationTest.java create mode 100644 server/src/test/java/com/github/jinba1/cuckoodb/server/web/UploadLimitIntegrationTest.java diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 848ffc2..6f40e66 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -20,10 +20,10 @@ jobs: path: ~/.m2/repository key: ${{ runner.os }}-m2-${{ hashFiles('**/pom.xml') }} restore-keys: ${{ runner.os }}-m2 - - name: Build - run: ./mvnw clean compile - - name: Test - run: ./mvnw test + - name: Build, test and package + # package subsumes compile + test for both modules and exercises the + # spring-boot-maven-plugin repackage, so a broken boot jar can't ship. + run: ./mvnw clean package - name: Upload coverage to Codecov uses: codecov/codecov-action@v5 with: diff --git a/engine/pom.xml b/engine/pom.xml index dc1745d..af534c9 100644 --- a/engine/pom.xml +++ b/engine/pom.xml @@ -28,21 +28,25 @@ org.junit.jupiter junit-jupiter-api + 5.10.2 test org.junit.jupiter junit-jupiter-engine + 5.10.2 test org.openjdk.jmh jmh-core + ${jmh.version} test org.openjdk.jmh jmh-generator-annprocess + ${jmh.version} test diff --git a/engine/src/main/java/com/github/jinba1/cuckoodb/BudgetKind.java b/engine/src/main/java/com/github/jinba1/cuckoodb/BudgetKind.java new file mode 100644 index 0000000..9aec563 --- /dev/null +++ b/engine/src/main/java/com/github/jinba1/cuckoodb/BudgetKind.java @@ -0,0 +1,15 @@ +package com.github.jinba1.cuckoodb; + +/** + * Which limit of a {@link QueryBudget} was breached. The engine throws one + * {@link QueryBudgetExceededException} type for both, but downstream surfaces treat them + * differently: a network gateway maps a {@link #TUPLES} breach to "retry with a narrower + * scope" (429) and a {@link #TIME} breach to "the query took too long" (504). Carried as a + * field so consumers classify by enum, never by parsing the message. + */ +public enum BudgetKind { + /** The tuple-count limit was exceeded. */ + TUPLES, + /** The wall-clock time limit was exceeded. */ + TIME +} diff --git a/engine/src/main/java/com/github/jinba1/cuckoodb/QueryBudget.java b/engine/src/main/java/com/github/jinba1/cuckoodb/QueryBudget.java index 24c9413..db44bd2 100644 --- a/engine/src/main/java/com/github/jinba1/cuckoodb/QueryBudget.java +++ b/engine/src/main/java/com/github/jinba1/cuckoodb/QueryBudget.java @@ -39,13 +39,13 @@ public void charge() { } processed++; if (maxTuples != null && processed > maxTuples) { - throw new QueryBudgetExceededException( + throw new QueryBudgetExceededException(BudgetKind.TUPLES, "Tuple budget exceeded: limit " + maxTuples + ", query processed " + processed + " tuples"); } // >= so an already-expired deadline (timeout 0) trips regardless of clock resolution if (timeoutMs != null && System.nanoTime() >= deadlineNanos) { - throw new QueryBudgetExceededException( + throw new QueryBudgetExceededException(BudgetKind.TIME, "Time budget exceeded: limit " + timeoutMs + " ms"); } } @@ -67,7 +67,7 @@ public void checkDeadline() { deadlineNanos = System.nanoTime() + timeoutMs * 1_000_000; } if (System.nanoTime() >= deadlineNanos) { - throw new QueryBudgetExceededException( + throw new QueryBudgetExceededException(BudgetKind.TIME, "Time budget exceeded: limit " + timeoutMs + " ms"); } } diff --git a/engine/src/main/java/com/github/jinba1/cuckoodb/QueryBudgetExceededException.java b/engine/src/main/java/com/github/jinba1/cuckoodb/QueryBudgetExceededException.java index c2dfa08..cd271a2 100644 --- a/engine/src/main/java/com/github/jinba1/cuckoodb/QueryBudgetExceededException.java +++ b/engine/src/main/java/com/github/jinba1/cuckoodb/QueryBudgetExceededException.java @@ -5,11 +5,22 @@ * Subclass of QueryExecutionException so existing error handling paths apply. */ public class QueryBudgetExceededException extends QueryExecutionException { + + private final BudgetKind kind; + /** + * @param kind which limit was breached (tuples vs time); lets a gateway map the same + * engine exception to different HTTP statuses without parsing the message * @param message which limit was exceeded and by how much; the error code is * always {@link ErrorCode#BUDGET_EXCEEDED} */ - public QueryBudgetExceededException(String message) { + public QueryBudgetExceededException(BudgetKind kind, String message) { super(ErrorCode.BUDGET_EXCEEDED, message); + this.kind = kind; + } + + /** Which budget limit was breached. */ + public BudgetKind kind() { + return kind; } } diff --git a/engine/src/test/java/com/github/jinba1/cuckoodb/QueryBudgetTest.java b/engine/src/test/java/com/github/jinba1/cuckoodb/QueryBudgetTest.java index 4444b61..6ff75d9 100644 --- a/engine/src/test/java/com/github/jinba1/cuckoodb/QueryBudgetTest.java +++ b/engine/src/test/java/com/github/jinba1/cuckoodb/QueryBudgetTest.java @@ -63,6 +63,32 @@ public void exceptionIsAQueryExecutionException() { assertTrue(QueryExecutionException.class.isAssignableFrom(QueryBudgetExceededException.class)); } + @Test + public void tupleBreachReportsKindTuples() { + // The REST layer maps a TUPLES breach to 429 and a TIME breach to 504, so the kind + // must be programmatically distinguishable, not parsed from the message text. + QueryBudget budget = new QueryBudget(0L, null); + QueryBudgetExceededException ex = + assertThrows(QueryBudgetExceededException.class, budget::charge); + assertEquals(BudgetKind.TUPLES, ex.kind()); + } + + @Test + public void timeBreachInChargeReportsKindTime() { + QueryBudget budget = new QueryBudget(null, 0L); + QueryBudgetExceededException ex = + assertThrows(QueryBudgetExceededException.class, budget::charge); + assertEquals(BudgetKind.TIME, ex.kind()); + } + + @Test + public void timeBreachInCheckDeadlineReportsKindTime() { + QueryBudget budget = new QueryBudget(null, 0L); + QueryBudgetExceededException ex = + assertThrows(QueryBudgetExceededException.class, budget::checkDeadline); + assertEquals(BudgetKind.TIME, ex.kind()); + } + @Test public void checkDeadlineWithZeroTimeoutTripsImmediately() { QueryBudget budget = new QueryBudget(null, 0L); diff --git a/pom.xml b/pom.xml index 2cb4f92..bba38a7 100644 --- a/pom.xml +++ b/pom.xml @@ -24,18 +24,14 @@ 1.37 + - - org.junit.jupiter - junit-jupiter-api - 5.10.2 - - - org.junit.jupiter - junit-jupiter-engine - 5.10.2 - com.github.jsqlparser jsqlparser @@ -46,16 +42,6 @@ commons-csv 1.14.1 - - org.openjdk.jmh - jmh-core - ${jmh.version} - - - org.openjdk.jmh - jmh-generator-annprocess - ${jmh.version} - diff --git a/server/pom.xml b/server/pom.xml index 2755430..9433ad2 100644 --- a/server/pom.xml +++ b/server/pom.xml @@ -16,16 +16,77 @@ cuckooDB Server + + 3.4.1 + 2.7.0 + + + + + + org.springframework.boot + spring-boot-dependencies + ${spring-boot.version} + pom + import + + + + com.github.jinba1 cuckoodb-engine ${project.version} + + org.springframework.boot + spring-boot-starter-web + + + org.springdoc + springdoc-openapi-starter-webmvc-ui + ${springdoc.version} + + + org.springframework.boot + spring-boot-starter-test + test + + + + + + org.apache.maven.plugins + maven-compiler-plugin + + + true + + + + org.springframework.boot + spring-boot-maven-plugin + ${spring-boot.version} + + + + repackage + + + + + + diff --git a/server/src/main/java/com/github/jinba1/cuckoodb/server/CuckooDbServerApplication.java b/server/src/main/java/com/github/jinba1/cuckoodb/server/CuckooDbServerApplication.java new file mode 100644 index 0000000..8658082 --- /dev/null +++ b/server/src/main/java/com/github/jinba1/cuckoodb/server/CuckooDbServerApplication.java @@ -0,0 +1,20 @@ +package com.github.jinba1.cuckoodb.server; + +import com.github.jinba1.cuckoodb.server.config.CuckooDbProperties; + +import org.springframework.boot.SpringApplication; +import org.springframework.boot.autoconfigure.SpringBootApplication; +import org.springframework.boot.context.properties.EnableConfigurationProperties; + +/** + * Entry point for the cuckooDB REST gateway: a guarded, read-only-by-construction, + * resource-budgeted HTTP front door onto the in-memory query engine. + */ +@SpringBootApplication +@EnableConfigurationProperties(CuckooDbProperties.class) +public class CuckooDbServerApplication { + + public static void main(String[] args) { + SpringApplication.run(CuckooDbServerApplication.class, args); + } +} diff --git a/server/src/main/java/com/github/jinba1/cuckoodb/server/ServerPlaceholder.java b/server/src/main/java/com/github/jinba1/cuckoodb/server/ServerPlaceholder.java deleted file mode 100644 index d0e8f42..0000000 --- a/server/src/main/java/com/github/jinba1/cuckoodb/server/ServerPlaceholder.java +++ /dev/null @@ -1,28 +0,0 @@ -package com.github.jinba1.cuckoodb.server; - -import com.github.jinba1.cuckoodb.QueryResult; - -/** - * Placeholder for the cuckooDB server module. - * - *

The Spring Boot REST gateway is added in a later change. This class exists - * only so that the {@code cuckoodb-server} module compiles against an engine type, - * proving the {@code server -> engine} Maven dependency is wired correctly. A - * broken reactor dependency would fail compilation here rather than passing silently. - */ -public final class ServerPlaceholder { - - private ServerPlaceholder() { - } - - /** - * References an engine type ({@link QueryResult}) so the engine dependency is - * exercised at compile time, not merely declared in the POM. - * - * @return a short description naming the linked engine type - */ - public static String describe() { - return "cuckooDB server skeleton; REST gateway added later. Engine type linked: " - + QueryResult.class.getSimpleName(); - } -} diff --git a/server/src/main/java/com/github/jinba1/cuckoodb/server/audit/AuditEvent.java b/server/src/main/java/com/github/jinba1/cuckoodb/server/audit/AuditEvent.java new file mode 100644 index 0000000..86f686b --- /dev/null +++ b/server/src/main/java/com/github/jinba1/cuckoodb/server/audit/AuditEvent.java @@ -0,0 +1,42 @@ +package com.github.jinba1.cuckoodb.server.audit; + +/** + * One auditable query attempt, captured at the {@code QueryService} choke point after planning + * (so the plan is in scope and a later governance phase need not re-plan). The shape is fixed now + * even though the sink is a no-op, so wiring a persistent audit log later is a sink swap, not an + * API change. + * + *

Exactly one of {@code rowCount} / {@code errorCode} is meaningful per outcome: + * a success carries {@code rowCount} (and {@code truncated}); a failure carries {@code errorCode}. + * + * @param principal the caller label (anonymous until authentication is added) + * @param sql the submitted SQL text + * @param explainText the rendered plan for an EXPLAIN request, else null + * @param outcome SUCCESS / EXPLAIN / ERROR + * @param rowCount rows returned on success (null otherwise) + * @param truncated whether a LIMIT truncated the result (false for explain/error) + * @param errorCode the engine {@code ErrorCode} name on failure (null otherwise) + */ +public record AuditEvent( + String principal, + String sql, + String explainText, + Outcome outcome, + Long rowCount, + boolean truncated, + String errorCode) { + + public enum Outcome { SUCCESS, EXPLAIN, ERROR } + + public static AuditEvent success(String principal, String sql, long rowCount, boolean truncated) { + return new AuditEvent(principal, sql, null, Outcome.SUCCESS, rowCount, truncated, null); + } + + public static AuditEvent explain(String principal, String sql, String explainText) { + return new AuditEvent(principal, sql, explainText, Outcome.EXPLAIN, null, false, null); + } + + public static AuditEvent error(String principal, String sql, String errorCode) { + return new AuditEvent(principal, sql, null, Outcome.ERROR, null, false, errorCode); + } +} diff --git a/server/src/main/java/com/github/jinba1/cuckoodb/server/audit/AuditSink.java b/server/src/main/java/com/github/jinba1/cuckoodb/server/audit/AuditSink.java new file mode 100644 index 0000000..7fef062 --- /dev/null +++ b/server/src/main/java/com/github/jinba1/cuckoodb/server/audit/AuditSink.java @@ -0,0 +1,10 @@ +package com.github.jinba1.cuckoodb.server.audit; + +/** + * Receives one {@link AuditEvent} per query attempt. The interface (and the event shape) are + * defined now so the {@code QueryService} call site is permanent; a later governance phase + * supplies a persistent implementation. The default {@link NoOpAuditSink} discards events. + */ +public interface AuditSink { + void record(AuditEvent event); +} diff --git a/server/src/main/java/com/github/jinba1/cuckoodb/server/audit/NoOpAuditSink.java b/server/src/main/java/com/github/jinba1/cuckoodb/server/audit/NoOpAuditSink.java new file mode 100644 index 0000000..a9c3c44 --- /dev/null +++ b/server/src/main/java/com/github/jinba1/cuckoodb/server/audit/NoOpAuditSink.java @@ -0,0 +1,20 @@ +package com.github.jinba1.cuckoodb.server.audit; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.stereotype.Component; + +/** + * Default {@link AuditSink}: logs each event at debug and otherwise discards it. A later + * governance phase replaces this with a persistent sink (a bean override, no call-site change). + */ +@Component +public class NoOpAuditSink implements AuditSink { + + private static final Logger log = LoggerFactory.getLogger(NoOpAuditSink.class); + + @Override + public void record(AuditEvent event) { + log.debug("audit: {}", event); + } +} diff --git a/server/src/main/java/com/github/jinba1/cuckoodb/server/catalog/CatalogFacade.java b/server/src/main/java/com/github/jinba1/cuckoodb/server/catalog/CatalogFacade.java new file mode 100644 index 0000000..61dd20a --- /dev/null +++ b/server/src/main/java/com/github/jinba1/cuckoodb/server/catalog/CatalogFacade.java @@ -0,0 +1,76 @@ +package com.github.jinba1.cuckoodb.server.catalog; + +import com.github.jinba1.cuckoodb.ColumnType; +import com.github.jinba1.cuckoodb.DBCatalog; +import com.github.jinba1.cuckoodb.TableMeta; + +import org.springframework.stereotype.Component; + +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; + +/** + * The server's only doorway to the engine's static {@link DBCatalog} singleton. Wrapping it + * in a Spring bean gives controllers a mockable seam (the static singleton cannot be mocked) + * and keeps every catalog read/write in one auditable place. The bean holds no state of its + * own — the catalog instance lifecycle is owned by {@code CatalogInitializer}. + */ +@Component +public class CatalogFacade { + + /** Table names currently in the catalog, sorted. */ + public List tableNames() { + return DBCatalog.getInstance().getTableNames(); + } + + /** Whether a table of this exact name is registered. */ + public boolean exists(String name) { + return DBCatalog.getInstance().tableExists(name); + } + + /** Current registered-table count, for the upload table-count cap. */ + public int tableCount() { + return DBCatalog.getInstance().getTableNames().size(); + } + + /** + * The static, catalog-authoritative typed schema of a base table, in column order, or + * {@code null} if the table is absent. Schema discovery must use this — never a query + * result's best-effort inferred types, which are null on an empty result. + */ + public List columnsOf(String name) { + TableMeta meta = DBCatalog.getInstance().getTableMeta(name); + if (meta == null) { + return null; + } + Map schema = meta.schema(); + List types = meta.types(); + String[] byIndex = new String[types.size()]; + for (Map.Entry e : schema.entrySet()) { + int idx = e.getValue(); + if (idx >= 0 && idx < byIndex.length) { + byIndex[idx] = e.getKey(); + } + } + List columns = new ArrayList<>(types.size()); + for (int i = 0; i < types.size(); i++) { + columns.add(new TableColumn(byIndex[i], types.get(i))); + } + return columns; + } + + /** + * Registers a table from an already-written CSV file. Returns {@code false} when the name + * is already taken (the 409 signal); throws {@code QueryExecutionException(DATA_ERROR)} for + * a malformed CSV. The file must outlive every query that scans the table. + */ + public boolean register(String name, Path csv) { + return DBCatalog.getInstance().registerTable(name, csv); + } + + /** One column of a base table's static schema: bare name + catalog-inferred type. */ + public record TableColumn(String name, ColumnType type) { + } +} diff --git a/server/src/main/java/com/github/jinba1/cuckoodb/server/config/CatalogInitializer.java b/server/src/main/java/com/github/jinba1/cuckoodb/server/config/CatalogInitializer.java new file mode 100644 index 0000000..d1413b3 --- /dev/null +++ b/server/src/main/java/com/github/jinba1/cuckoodb/server/config/CatalogInitializer.java @@ -0,0 +1,57 @@ +package com.github.jinba1.cuckoodb.server.config; + +import com.github.jinba1.cuckoodb.DBCatalog; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.boot.ApplicationArguments; +import org.springframework.boot.ApplicationRunner; +import org.springframework.stereotype.Component; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; + +/** + * Owns the catalog lifecycle under Spring. The engine's {@link DBCatalog} is a static + * singleton whose {@code initDBCatalog} silently no-ops a second call and whose + * {@code resetDBCatalog} nulls the instance with no reader coordination — both fight Spring's + * (and the test harness's) context lifecycle. + * + *

So this runner performs the one deterministic seeding for a context: {@code reset} then + * {@code init}, exactly once, before any request can arrive. At production boot the reset is a + * no-op (the singleton starts null); in tests it gives each freshly-built context a catalog + * seeded purely from its own configured {@code data-dir}, with no leakage from a prior + * context's singleton. Integration tests therefore must NOT call {@code resetDBCatalog} + * themselves and should isolate contexts with {@code @DirtiesContext}. + */ +@Component +public class CatalogInitializer implements ApplicationRunner { + + private static final Logger log = LoggerFactory.getLogger(CatalogInitializer.class); + + private final CuckooDbProperties properties; + + public CatalogInitializer(CuckooDbProperties properties) { + this.properties = properties; + } + + @Override + public void run(ApplicationArguments args) throws IOException { + Path workDir = Path.of(properties.workDir()); + Files.createDirectories(workDir); + log.info("Upload work dir: {}", workDir.toAbsolutePath()); + + DBCatalog.resetDBCatalog(); + String dataDir = properties.dataDir(); + if (dataDir != null && !dataDir.isBlank()) { + DBCatalog.initDBCatalog(dataDir); + log.info("Catalog seeded from data dir '{}': {} table(s)", + dataDir, DBCatalog.getInstance().getTableNames().size()); + } else { + // No seed dir: create an empty catalog so registerTable has an instance to write to. + DBCatalog.getInstance(); + log.info("No cuckoodb.data-dir configured; catalog starts empty (uploads only)"); + } + } +} diff --git a/server/src/main/java/com/github/jinba1/cuckoodb/server/config/CuckooDbProperties.java b/server/src/main/java/com/github/jinba1/cuckoodb/server/config/CuckooDbProperties.java new file mode 100644 index 0000000..eceecf6 --- /dev/null +++ b/server/src/main/java/com/github/jinba1/cuckoodb/server/config/CuckooDbProperties.java @@ -0,0 +1,81 @@ +package com.github.jinba1.cuckoodb.server.config; + +import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.boot.context.properties.NestedConfigurationProperty; + +/** + * Server-side configuration, bound from the {@code cuckoodb.*} property namespace. All + * resource bounds (budget caps, upload size, table count, concurrency) live here so they are + * tunable per deployment without code changes, and every default is conservative. + * + * @param dataDir directory whose {@code data/} subdir is loaded into the catalog at + * startup; {@code null} leaves the catalog empty (uploads only) + * @param workDir directory where uploaded CSVs are persisted for the process lifetime; + * must NOT be an auto-deleted temp dir because joins re-open table files + * @param maxConcurrentQueries permits for the query semaphore; a miss returns 429 + * @param query per-query budget defaults and hard caps + * @param upload upload feature flag and limits + */ +@ConfigurationProperties(prefix = "cuckoodb") +public record CuckooDbProperties( + String dataDir, + String workDir, + Integer maxConcurrentQueries, + @NestedConfigurationProperty Query query, + @NestedConfigurationProperty Upload upload) { + + public CuckooDbProperties { + if (workDir == null || workDir.isBlank()) { + workDir = System.getProperty("java.io.tmpdir") + "/cuckoodb-work"; + } + if (maxConcurrentQueries == null || maxConcurrentQueries < 1) { + maxConcurrentQueries = Runtime.getRuntime().availableProcessors(); + } + if (query == null) { + query = new Query(null, null, null, null); + } + if (upload == null) { + upload = new Upload(null, null, null); + } + } + + /** + * Per-query budget settings. The server always attaches a budget (the engine has none by + * default = unlimited), so both a tuple and a time bound are required: a time-only budget + * would not cap a Sort/HashJoin build heap. + * + * @param maxTuplesDefault tuples charged when the request omits {@code maxTuples} + * @param maxTuplesCap hard ceiling; a larger request is clamped down to this + * @param timeoutMsDefault wall-clock ms when the request omits {@code timeoutMs} + * @param timeoutMsCap hard ceiling; a larger request is clamped down to this + */ + public record Query(Long maxTuplesDefault, Long maxTuplesCap, + Long timeoutMsDefault, Long timeoutMsCap) { + public Query { + if (maxTuplesDefault == null) maxTuplesDefault = 100_000L; + if (maxTuplesCap == null) maxTuplesCap = 1_000_000L; + if (timeoutMsDefault == null) timeoutMsDefault = 5_000L; + if (timeoutMsCap == null) timeoutMsCap = 30_000L; + } + } + + /** + * Upload feature settings. Disabled by default so the write surface stays closed until a + * deployment opts in (and a later governance phase is added). + * + * @param enabled whether {@code POST /tables/{name}} is mounted at all + * @param maxBytes per-upload size cap in bytes; a larger body returns 413 + * @param maxTables process-wide table count ceiling; over it returns 507 + */ + public record Upload(Boolean enabled, Long maxBytes, Integer maxTables) { + /** Absolute ceiling on the configurable per-upload size cap. */ + public static final long HARD_MAX_BYTES = 50L * 1024 * 1024; + + public Upload { + if (enabled == null) enabled = false; + if (maxBytes == null) maxBytes = 10L * 1024 * 1024; + if (maxBytes > HARD_MAX_BYTES) maxBytes = HARD_MAX_BYTES; + if (maxTables == null || maxTables < 1) maxTables = 100; + } + } +} diff --git a/server/src/main/java/com/github/jinba1/cuckoodb/server/config/OpenApiConfig.java b/server/src/main/java/com/github/jinba1/cuckoodb/server/config/OpenApiConfig.java new file mode 100644 index 0000000..40af3eb --- /dev/null +++ b/server/src/main/java/com/github/jinba1/cuckoodb/server/config/OpenApiConfig.java @@ -0,0 +1,27 @@ +package com.github.jinba1.cuckoodb.server.config; + +import io.swagger.v3.oas.models.OpenAPI; +import io.swagger.v3.oas.models.info.Info; + +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; + +/** + * OpenAPI document metadata. springdoc auto-generates the schema and serves Swagger UI at + * {@code /swagger-ui.html} and the spec at {@code /v3/api-docs}; this only supplies the + * title/description/version so the contract documents the gateway's read-only, budgeted intent. + */ +@Configuration +public class OpenApiConfig { + + @Bean + public OpenAPI cuckooDbOpenApi() { + return new OpenAPI().info(new Info() + .title("cuckooDB REST API") + .version("1.0.0") + .description("A guarded, read-only-by-construction, resource-budgeted HTTP front " + + "door onto the cuckooDB in-memory query engine. POST /queries executes " + + "SQL synchronously; the /tables endpoints expose catalog schema and " + + "(opt-in) CSV upload.")); + } +} diff --git a/server/src/main/java/com/github/jinba1/cuckoodb/server/query/ApiPrincipalResolver.java b/server/src/main/java/com/github/jinba1/cuckoodb/server/query/ApiPrincipalResolver.java new file mode 100644 index 0000000..09c4979 --- /dev/null +++ b/server/src/main/java/com/github/jinba1/cuckoodb/server/query/ApiPrincipalResolver.java @@ -0,0 +1,37 @@ +package com.github.jinba1.cuckoodb.server.query; + +import org.springframework.stereotype.Component; + +import jakarta.servlet.http.HttpServletRequest; + +import java.util.Optional; + +/** + * Resolves the calling principal from the {@code X-Api-Key} header. Today it only echoes the + * key (or anonymous); a later governance phase turns this into real authentication and per-key + * policy. It exists now so the audit trail and budget policy can be keyed on a principal without + * a later signature change. + */ +@Component +public class ApiPrincipalResolver { + + /** Header carrying the (currently unauthenticated) API key. */ + public static final String API_KEY_HEADER = "X-Api-Key"; + + /** Principal label used when no API key is presented. */ + public static final String ANONYMOUS = "anonymous"; + + /** The presented API key, if any — not yet validated. */ + public Optional apiKey(HttpServletRequest request) { + String key = request.getHeader(API_KEY_HEADER); + if (key == null || key.isBlank()) { + return Optional.empty(); + } + return Optional.of(key.strip()); + } + + /** A principal label for audit/budget keying; {@link #ANONYMOUS} when no key is presented. */ + public String resolve(HttpServletRequest request) { + return apiKey(request).orElse(ANONYMOUS); + } +} diff --git a/server/src/main/java/com/github/jinba1/cuckoodb/server/query/BudgetPolicy.java b/server/src/main/java/com/github/jinba1/cuckoodb/server/query/BudgetPolicy.java new file mode 100644 index 0000000..652ddd4 --- /dev/null +++ b/server/src/main/java/com/github/jinba1/cuckoodb/server/query/BudgetPolicy.java @@ -0,0 +1,51 @@ +package com.github.jinba1.cuckoodb.server.query; + +import com.github.jinba1.cuckoodb.server.config.CuckooDbProperties; + +import org.springframework.stereotype.Component; + +/** + * Turns an optional client budget request into a concrete, always-present budget: each + * dimension defaults when the client omits it and is clamped down to the configured hard cap + * (the cap always wins). The result is never null and never has a null dimension, so the + * engine's "null = unlimited" path is unreachable through the server — the fail-closed + * guarantee. This is also the seam where future per-principal caps will apply. + */ +@Component +public class BudgetPolicy { + + private final CuckooDbProperties.Query query; + + public BudgetPolicy(CuckooDbProperties properties) { + this.query = properties.query(); + } + + /** + * @param requestedMaxTuples client's tuple bound, or null to take the default + * @param requestedTimeoutMs client's time bound, or null to take the default + * @return a fully-populated budget, both dimensions non-null and within the hard caps + * @throws IllegalArgumentException if a client supplies a non-positive bound + */ + public ClampedBudget clamp(Long requestedMaxTuples, Long requestedTimeoutMs) { + long maxTuples = clampDimension(requestedMaxTuples, + query.maxTuplesDefault(), query.maxTuplesCap(), "maxTuples"); + long timeoutMs = clampDimension(requestedTimeoutMs, + query.timeoutMsDefault(), query.timeoutMsCap(), "timeoutMs"); + return new ClampedBudget(maxTuples, timeoutMs); + } + + private static long clampDimension(Long requested, long fallback, long cap, String name) { + if (requested == null) { + return fallback; + } + if (requested <= 0) { + throw new IllegalArgumentException( + name + " must be a positive integer; got " + requested); + } + return Math.min(requested, cap); + } + + /** A concrete budget with both dimensions resolved and bounded; never null-valued. */ + public record ClampedBudget(long maxTuples, long timeoutMs) { + } +} diff --git a/server/src/main/java/com/github/jinba1/cuckoodb/server/query/ConcurrencyLimitExceededException.java b/server/src/main/java/com/github/jinba1/cuckoodb/server/query/ConcurrencyLimitExceededException.java new file mode 100644 index 0000000..d37de96 --- /dev/null +++ b/server/src/main/java/com/github/jinba1/cuckoodb/server/query/ConcurrencyLimitExceededException.java @@ -0,0 +1,11 @@ +package com.github.jinba1.cuckoodb.server.query; + +/** + * Thrown when the query concurrency semaphore is saturated. Maps to HTTP 429 with a + * {@code Retry-After} header — the condition is transient, so the client should retry shortly. + */ +public class ConcurrencyLimitExceededException extends RuntimeException { + public ConcurrencyLimitExceededException(String message) { + super(message); + } +} diff --git a/server/src/main/java/com/github/jinba1/cuckoodb/server/query/ConcurrencyLimiter.java b/server/src/main/java/com/github/jinba1/cuckoodb/server/query/ConcurrencyLimiter.java new file mode 100644 index 0000000..967ed05 --- /dev/null +++ b/server/src/main/java/com/github/jinba1/cuckoodb/server/query/ConcurrencyLimiter.java @@ -0,0 +1,54 @@ +package com.github.jinba1.cuckoodb.server.query; + +import com.github.jinba1.cuckoodb.server.config.CuckooDbProperties; + +import org.springframework.stereotype.Component; + +import java.util.concurrent.Semaphore; +import java.util.concurrent.TimeUnit; +import java.util.function.Supplier; + +/** + * Bounds the number of queries executing at once with a single fair semaphore. The engine is + * CPU-bound and holds intermediate results in heap, so unbounded concurrency would exhaust + * memory and thrash the CPU; capping it keeps each admitted query's budget meaningful. A miss + * is surfaced as a 429 rather than queued indefinitely, so a saturated server fails fast. + */ +@Component +public class ConcurrencyLimiter { + + private final Semaphore semaphore; + + public ConcurrencyLimiter(CuckooDbProperties properties) { + this.semaphore = new Semaphore(properties.maxConcurrentQueries(), true); + } + + /** + * Runs {@code work} while holding one permit, releasing it in a finally. Acquisition is + * non-blocking: if no permit is free the work never starts. + * @throws ConcurrencyLimitExceededException if no permit could be acquired + */ + public T withPermit(Supplier work) { + boolean acquired; + try { + acquired = semaphore.tryAcquire(0, TimeUnit.MILLISECONDS); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new ConcurrencyLimitExceededException("Query admission interrupted"); + } + if (!acquired) { + throw new ConcurrencyLimitExceededException( + "Server is at its query concurrency limit; retry shortly"); + } + try { + return work.get(); + } finally { + semaphore.release(); + } + } + + /** Permits currently available — for tests and diagnostics. */ + public int availablePermits() { + return semaphore.availablePermits(); + } +} diff --git a/server/src/main/java/com/github/jinba1/cuckoodb/server/query/QueryService.java b/server/src/main/java/com/github/jinba1/cuckoodb/server/query/QueryService.java new file mode 100644 index 0000000..bf88672 --- /dev/null +++ b/server/src/main/java/com/github/jinba1/cuckoodb/server/query/QueryService.java @@ -0,0 +1,84 @@ +package com.github.jinba1.cuckoodb.server.query; + +import com.github.jinba1.cuckoodb.CuckooDB; +import com.github.jinba1.cuckoodb.ErrorCode; +import com.github.jinba1.cuckoodb.PlannedQuery; +import com.github.jinba1.cuckoodb.QueryBudget; +import com.github.jinba1.cuckoodb.QueryConfig; +import com.github.jinba1.cuckoodb.QueryExecutionException; +import com.github.jinba1.cuckoodb.QueryPlanner; +import com.github.jinba1.cuckoodb.QueryResultSet; +import com.github.jinba1.cuckoodb.operator.Operator; +import com.github.jinba1.cuckoodb.server.audit.AuditEvent; +import com.github.jinba1.cuckoodb.server.audit.AuditSink; + +import org.springframework.stereotype.Service; + +/** + * The single choke point through which every query passes, so governance, budgeting, + * concurrency bounding, and audit cannot be bypassed by any controller. + * + *

The pipeline per request: acquire a concurrency permit → plan the SQL (String overload) → + * branch EXPLAIN (returns plan text, zero tuple budget) → clamp and attach a fail-closed budget + * → drain in memory → audit. A budget is always attached for an executed query: the + * engine treats a null budget as unlimited, and {@link BudgetPolicy} never yields null, so the + * unbounded path is unreachable here. + */ +@Service +public class QueryService { + + private final BudgetPolicy budgetPolicy; + private final ConcurrencyLimiter concurrencyLimiter; + private final AuditSink auditSink; + + public QueryService(BudgetPolicy budgetPolicy, ConcurrencyLimiter concurrencyLimiter, + AuditSink auditSink) { + this.budgetPolicy = budgetPolicy; + this.concurrencyLimiter = concurrencyLimiter; + this.auditSink = auditSink; + } + + /** + * Plans and (unless EXPLAIN) executes one query under a clamped budget, holding a + * concurrency permit for the whole call. + * + * @param sql the SQL text (optionally EXPLAIN-prefixed); never a file path + * @param maxTuples client tuple bound, or null to take the configured default + * @param timeoutMs client time bound, or null to take the configured default + * @param principal caller label for the audit trail + * @return the executed result set or the EXPLAIN plan text + * @throws QueryExecutionException (classified by {@code ErrorCode}) on any engine failure + */ + public QueryServiceResult execute(String sql, Long maxTuples, Long timeoutMs, String principal) { + return concurrencyLimiter.withPermit(() -> runPlanned(sql, maxTuples, timeoutMs, principal)); + } + + private QueryServiceResult runPlanned(String sql, Long maxTuples, Long timeoutMs, + String principal) { + try { + PlannedQuery planned = QueryPlanner.planSql(sql, QueryConfig.defaults()); + + if (planned.explainText() != null) { + auditSink.record(AuditEvent.explain(principal, sql, planned.explainText())); + return QueryServiceResult.explain(planned.explainText()); + } + + BudgetPolicy.ClampedBudget budget = budgetPolicy.clamp(maxTuples, timeoutMs); + Operator root = planned.root(); + root.attachBudget(new QueryBudget(budget.maxTuples(), budget.timeoutMs())); + + QueryResultSet resultSet = CuckooDB.executeToResultSet(root); + auditSink.record(AuditEvent.success( + principal, sql, resultSet.rows().size(), resultSet.truncated())); + return QueryServiceResult.of(resultSet); + } catch (QueryExecutionException e) { + auditSink.record(AuditEvent.error(principal, sql, e.code().name())); + throw e; + } catch (RuntimeException e) { + // An unchecked failure (engine bug, or a budget-validation IllegalArgumentException) + // must still leave an audit trail — the choke point's audit cannot be bypassed. + auditSink.record(AuditEvent.error(principal, sql, ErrorCode.INTERNAL.name())); + throw e; + } + } +} diff --git a/server/src/main/java/com/github/jinba1/cuckoodb/server/query/QueryServiceResult.java b/server/src/main/java/com/github/jinba1/cuckoodb/server/query/QueryServiceResult.java new file mode 100644 index 0000000..1aca7c5 --- /dev/null +++ b/server/src/main/java/com/github/jinba1/cuckoodb/server/query/QueryServiceResult.java @@ -0,0 +1,26 @@ +package com.github.jinba1.cuckoodb.server.query; + +import com.github.jinba1.cuckoodb.QueryResultSet; + +/** + * The two shapes a successful {@code QueryService} call can produce: an executed result set, + * or EXPLAIN plan text (no execution). Exactly one field is non-null. Keeping them in one + * return type lets the controller branch on {@link #isExplain()} without a second service call. + * + * @param resultSet the materialized rows for an executed query, or null for EXPLAIN + * @param explainText the rendered plan for an EXPLAIN request, or null for an executed query + */ +public record QueryServiceResult(QueryResultSet resultSet, String explainText) { + + public static QueryServiceResult of(QueryResultSet resultSet) { + return new QueryServiceResult(resultSet, null); + } + + public static QueryServiceResult explain(String explainText) { + return new QueryServiceResult(null, explainText); + } + + public boolean isExplain() { + return explainText != null; + } +} diff --git a/server/src/main/java/com/github/jinba1/cuckoodb/server/web/GlobalExceptionHandler.java b/server/src/main/java/com/github/jinba1/cuckoodb/server/web/GlobalExceptionHandler.java new file mode 100644 index 0000000..61f1bcb --- /dev/null +++ b/server/src/main/java/com/github/jinba1/cuckoodb/server/web/GlobalExceptionHandler.java @@ -0,0 +1,157 @@ +package com.github.jinba1.cuckoodb.server.web; + +import com.github.jinba1.cuckoodb.BudgetKind; +import com.github.jinba1.cuckoodb.ErrorCode; +import com.github.jinba1.cuckoodb.QueryBudgetExceededException; +import com.github.jinba1.cuckoodb.QueryExecutionException; +import com.github.jinba1.cuckoodb.server.query.ConcurrencyLimitExceededException; +import com.github.jinba1.cuckoodb.server.web.dto.ErrorResponse; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpStatus; +import org.springframework.http.HttpStatusCode; +import org.springframework.http.ResponseEntity; +import org.springframework.http.converter.HttpMessageNotReadableException; +import org.springframework.web.HttpMediaTypeNotSupportedException; +import org.springframework.web.bind.annotation.ExceptionHandler; +import org.springframework.web.bind.annotation.RestControllerAdvice; +import org.springframework.web.context.request.WebRequest; +import org.springframework.web.servlet.mvc.method.annotation.ResponseEntityExceptionHandler; + +import java.util.UUID; + +/** + * Translates every failure into the uniform {@code {errorCode, message}} body with the right + * HTTP status. Engine {@link ErrorCode}s map to 4xx where the caller can act and 5xx only + * for genuine engine faults. Two rules protect the client: a budget breach splits by + * {@link BudgetKind} (tuples → 429+Retry-After, time → 504), and every 5xx returns a generic + * message plus a correlation {@code errorId} while the full detail (which may embed filesystem + * paths) is logged server-side only — stack traces are never serialized. + * + *

Extends {@link ResponseEntityExceptionHandler} so Spring's own request exceptions (unknown + * route, wrong method, etc.) keep their correct statuses instead of collapsing into the 500 + * fallback; only the media-type and unreadable-body cases are overridden to use our body shape. + */ +@RestControllerAdvice +public class GlobalExceptionHandler extends ResponseEntityExceptionHandler { + + private static final Logger log = LoggerFactory.getLogger(GlobalExceptionHandler.class); + + /** Retry-After value (seconds) for transient 429s. */ + private static final String RETRY_AFTER_SECONDS = "1"; + + /** Budget breach: distinguish a too-big result (429, retry narrower) from too-slow (504). */ + @ExceptionHandler(QueryBudgetExceededException.class) + public ResponseEntity handleBudget(QueryBudgetExceededException ex) { + ErrorResponse body = ErrorResponse.of(ex.code().name(), ex.getMessage()); + if (ex.kind() == BudgetKind.TIME) { + return ResponseEntity.status(HttpStatus.GATEWAY_TIMEOUT).body(body); + } + return retryable(HttpStatus.TOO_MANY_REQUESTS, body); + } + + /** All other classified engine failures. */ + @ExceptionHandler(QueryExecutionException.class) + public ResponseEntity handleEngine(QueryExecutionException ex) { + ErrorCode code = ex.code(); + ErrorResponse verbatim = ErrorResponse.of(code.name(), ex.getMessage()); + return switch (code) { + case PARSE_ERROR -> ResponseEntity.badRequest().body(verbatim); + case UNSUPPORTED_SQL, UNKNOWN_TABLE, UNKNOWN_COLUMN, TYPE_MISMATCH -> + ResponseEntity.unprocessableEntity().body(verbatim); + // A bare BUDGET_EXCEEDED (not the kinded subclass) is treated as the tuples case. + case BUDGET_EXCEEDED -> retryable(HttpStatus.TOO_MANY_REQUESTS, verbatim); + // Query-path data/internal faults can embed absolute paths — scrub to a generic 500. + case DATA_ERROR, INTERNAL -> internal(code, ex); + }; + } + + /** Concurrency saturation: transient, so 429 + Retry-After. */ + @ExceptionHandler(ConcurrencyLimitExceededException.class) + public ResponseEntity handleConcurrency(ConcurrencyLimitExceededException ex) { + return retryable(HttpStatus.TOO_MANY_REQUESTS, + ErrorResponse.of("CONCURRENCY_LIMIT", ex.getMessage())); + } + + @ExceptionHandler(TableNotFoundException.class) + public ResponseEntity handleTableNotFound(TableNotFoundException ex) { + return ResponseEntity.status(HttpStatus.NOT_FOUND) + .body(ErrorResponse.of("UNKNOWN_TABLE", ex.getMessage())); + } + + @ExceptionHandler(UploadDisabledException.class) + public ResponseEntity handleUploadDisabled(UploadDisabledException ex) { + return ResponseEntity.status(HttpStatus.NOT_FOUND) + .body(ErrorResponse.of("NOT_FOUND", ex.getMessage())); + } + + @ExceptionHandler(TableAlreadyExistsException.class) + public ResponseEntity handleTableExists(TableAlreadyExistsException ex) { + return ResponseEntity.status(HttpStatus.CONFLICT) + .body(ErrorResponse.of("TABLE_EXISTS", ex.getMessage())); + } + + @ExceptionHandler(PayloadTooLargeException.class) + public ResponseEntity handlePayloadTooLarge(PayloadTooLargeException ex) { + return ResponseEntity.status(HttpStatus.PAYLOAD_TOO_LARGE) + .body(ErrorResponse.of("PAYLOAD_TOO_LARGE", ex.getMessage())); + } + + @ExceptionHandler(TableLimitExceededException.class) + public ResponseEntity handleTableLimit(TableLimitExceededException ex) { + return ResponseEntity.status(HttpStatus.INSUFFICIENT_STORAGE) + .body(ErrorResponse.of("TABLE_LIMIT", ex.getMessage())); + } + + @ExceptionHandler(InvalidUploadException.class) + public ResponseEntity handleInvalidUpload(InvalidUploadException ex) { + return ResponseEntity.badRequest().body(ErrorResponse.of("DATA_ERROR", ex.getMessage())); + } + + @ExceptionHandler(IllegalArgumentException.class) + public ResponseEntity handleIllegalArgument(IllegalArgumentException ex) { + return ResponseEntity.badRequest().body(ErrorResponse.of("BAD_REQUEST", ex.getMessage())); + } + + /** Anything unclassified is an engine/server bug: scrub the body, log the detail. */ + @ExceptionHandler(Exception.class) + public ResponseEntity handleUnexpected(Exception ex) { + String errorId = UUID.randomUUID().toString(); + log.error("Unhandled exception errorId={}", errorId, ex); + return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) + .body(ErrorResponse.withId("INTERNAL", "Internal server error.", errorId)); + } + + @Override + protected ResponseEntity handleHttpMediaTypeNotSupported( + HttpMediaTypeNotSupportedException ex, HttpHeaders headers, + HttpStatusCode status, WebRequest request) { + return new ResponseEntity<>(ErrorResponse.of("UNSUPPORTED_MEDIA_TYPE", + "Unsupported Content-Type; this endpoint expects text/csv."), + HttpStatus.UNSUPPORTED_MEDIA_TYPE); + } + + @Override + protected ResponseEntity handleHttpMessageNotReadable( + HttpMessageNotReadableException ex, HttpHeaders headers, + HttpStatusCode status, WebRequest request) { + return new ResponseEntity<>( + ErrorResponse.of("BAD_REQUEST", "Malformed or missing request body."), + HttpStatus.BAD_REQUEST); + } + + private ResponseEntity internal(ErrorCode code, Exception ex) { + String errorId = UUID.randomUUID().toString(); + log.error("5xx [{}] errorId={}", code, errorId, ex); + return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) + .body(ErrorResponse.withId(code.name(), "Internal server error.", errorId)); + } + + private static ResponseEntity retryable(HttpStatus status, ErrorResponse body) { + return ResponseEntity.status(status) + .header(HttpHeaders.RETRY_AFTER, RETRY_AFTER_SECONDS) + .body(body); + } +} diff --git a/server/src/main/java/com/github/jinba1/cuckoodb/server/web/InvalidUploadException.java b/server/src/main/java/com/github/jinba1/cuckoodb/server/web/InvalidUploadException.java new file mode 100644 index 0000000..70cf9fd --- /dev/null +++ b/server/src/main/java/com/github/jinba1/cuckoodb/server/web/InvalidUploadException.java @@ -0,0 +1,12 @@ +package com.github.jinba1.cuckoodb.server.web; + +/** + * An uploaded CSV was malformed (bad header, ragged row, duplicate column, non-int in an + * int column). Maps to 400 — a client mistake, distinct from a query-path {@code DATA_ERROR} + * (500). The message is sanitized of any server filesystem path before it reaches the client. + */ +public class InvalidUploadException extends RuntimeException { + public InvalidUploadException(String message) { + super(message); + } +} diff --git a/server/src/main/java/com/github/jinba1/cuckoodb/server/web/PayloadTooLargeException.java b/server/src/main/java/com/github/jinba1/cuckoodb/server/web/PayloadTooLargeException.java new file mode 100644 index 0000000..d57a8fd --- /dev/null +++ b/server/src/main/java/com/github/jinba1/cuckoodb/server/web/PayloadTooLargeException.java @@ -0,0 +1,11 @@ +package com.github.jinba1.cuckoodb.server.web; + +/** + * An upload body exceeded the configured size cap while streaming. Maps to 413. Detected during + * the write so an oversized body is never fully buffered in memory. + */ +public class PayloadTooLargeException extends RuntimeException { + public PayloadTooLargeException(long maxBytes) { + super("Upload exceeds the maximum size of " + maxBytes + " bytes."); + } +} diff --git a/server/src/main/java/com/github/jinba1/cuckoodb/server/web/QueryController.java b/server/src/main/java/com/github/jinba1/cuckoodb/server/web/QueryController.java new file mode 100644 index 0000000..84abf9b --- /dev/null +++ b/server/src/main/java/com/github/jinba1/cuckoodb/server/web/QueryController.java @@ -0,0 +1,42 @@ +package com.github.jinba1.cuckoodb.server.web; + +import com.github.jinba1.cuckoodb.server.query.ApiPrincipalResolver; +import com.github.jinba1.cuckoodb.server.query.QueryService; +import com.github.jinba1.cuckoodb.server.query.QueryServiceResult; +import com.github.jinba1.cuckoodb.server.web.dto.QueryRequest; +import com.github.jinba1.cuckoodb.server.web.dto.QueryResponse; + +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.RestController; + +import jakarta.servlet.http.HttpServletRequest; + +/** + * {@code POST /queries}: plan and synchronously execute one read-only SQL query, returning a + * structured JSON result (or EXPLAIN plan text). All governance — budget, concurrency, audit — + * lives behind {@link QueryService}; this controller only adapts HTTP to that call. + */ +@RestController +@RequestMapping("/queries") +public class QueryController { + + private final QueryService queryService; + private final ApiPrincipalResolver principalResolver; + + public QueryController(QueryService queryService, ApiPrincipalResolver principalResolver) { + this.queryService = queryService; + this.principalResolver = principalResolver; + } + + @PostMapping + public QueryResponse query(@RequestBody QueryRequest request, HttpServletRequest httpRequest) { + String principal = principalResolver.resolve(httpRequest); + QueryServiceResult result = queryService.execute( + request.sql(), request.maxTuples(), request.timeoutMs(), principal); + return result.isExplain() + ? QueryResponse.fromExplain(result.explainText()) + : QueryResponse.fromResultSet(result.resultSet()); + } +} diff --git a/server/src/main/java/com/github/jinba1/cuckoodb/server/web/TableAlreadyExistsException.java b/server/src/main/java/com/github/jinba1/cuckoodb/server/web/TableAlreadyExistsException.java new file mode 100644 index 0000000..519321e --- /dev/null +++ b/server/src/main/java/com/github/jinba1/cuckoodb/server/web/TableAlreadyExistsException.java @@ -0,0 +1,11 @@ +package com.github.jinba1.cuckoodb.server.web; + +/** + * An upload targeted a table name already registered. Maps to 409 — the engine's atomic + * {@code putIfAbsent} lost, so the existing table is left untouched. + */ +public class TableAlreadyExistsException extends RuntimeException { + public TableAlreadyExistsException(String name) { + super("Table '" + name + "' already exists."); + } +} diff --git a/server/src/main/java/com/github/jinba1/cuckoodb/server/web/TableController.java b/server/src/main/java/com/github/jinba1/cuckoodb/server/web/TableController.java new file mode 100644 index 0000000..b79ee79 --- /dev/null +++ b/server/src/main/java/com/github/jinba1/cuckoodb/server/web/TableController.java @@ -0,0 +1,181 @@ +package com.github.jinba1.cuckoodb.server.web; + +import com.github.jinba1.cuckoodb.QueryExecutionException; +import com.github.jinba1.cuckoodb.server.catalog.CatalogFacade; +import com.github.jinba1.cuckoodb.server.config.CuckooDbProperties; +import com.github.jinba1.cuckoodb.server.web.dto.TableColumnDto; +import com.github.jinba1.cuckoodb.server.web.dto.TableSchemaResponse; +import com.github.jinba1.cuckoodb.server.web.dto.UploadResponse; + +import org.springframework.http.HttpStatus; +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.RequestMapping; +import org.springframework.web.bind.annotation.ResponseStatus; +import org.springframework.web.bind.annotation.RestController; + +import jakarta.servlet.http.HttpServletRequest; + +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardOpenOption; +import java.util.ArrayList; +import java.util.List; +import java.util.UUID; +import java.util.regex.Pattern; +import java.util.stream.Stream; + +/** + * Table catalog endpoints: list tables, describe a table's static typed schema, and (opt-in) + * upload a CSV as a process-lifetime table. The server is the only guard on the table + * name — the engine and {@code ScanOperator} use it verbatim — so every name is validated + * against a strict charset before any filesystem path is built, and the resolved path is + * asserted to stay within the work directory. + */ +@RestController +@RequestMapping("/tables") +public class TableController { + + /** The only table names the server will touch; blocks path traversal and odd characters. */ + private static final Pattern VALID_NAME = Pattern.compile("^[A-Za-z0-9_]{1,64}$"); + + private final CatalogFacade catalog; + private final CuckooDbProperties properties; + + public TableController(CatalogFacade catalog, CuckooDbProperties properties) { + this.catalog = catalog; + this.properties = properties; + } + + /** {@code GET /tables} — all registered table names, sorted. */ + @GetMapping + public List list() { + return catalog.tableNames(); + } + + /** {@code GET /tables/{name}} — a table's static, catalog-authoritative typed schema. */ + @GetMapping("/{name}") + public TableSchemaResponse describe(@PathVariable String name) { + validateName(name); + List columns = catalog.columnsOf(name); + if (columns == null) { + throw new TableNotFoundException(name); + } + return new TableSchemaResponse(name, toDto(columns)); + } + + /** + * {@code POST /tables/{name}} (text/csv) — register a CSV as a new table. Disabled by + * default (404 when off). Enforces, in order: name charset, table-count cap, streamed size + * cap, malformed-CSV rejection, and an atomic 409 on a name clash. The winning request's + * temp file becomes the table's permanent backing file (a losing or failing request deletes + * its temp), because the engine re-opens that file on every scan. + */ + @PostMapping(value = "/{name}", consumes = "text/csv") + @ResponseStatus(HttpStatus.CREATED) + public UploadResponse upload(@PathVariable String name, HttpServletRequest request) + throws IOException { + if (!properties.upload().enabled()) { + throw new UploadDisabledException(); + } + validateName(name); + if (catalog.tableCount() >= properties.upload().maxTables()) { + throw new TableLimitExceededException(properties.upload().maxTables()); + } + + Path target = resolveSafeTarget(name); + boolean keep = false; + try { + copyWithCap(request.getInputStream(), target, properties.upload().maxBytes()); + // Count rows BEFORE registering: a count failure here must abort cleanly (the finally + // deletes the temp), never leave the table live in the catalog while the client sees a + // 500 — that would make the table un-re-uploadable (409) yet reported as failed. + long rowCount = countDataRows(target); + + boolean registered; + try { + registered = catalog.register(name, target); + } catch (QueryExecutionException e) { + // Malformed CSV (DATA_ERROR) on the upload path is a client error (400), not a + // 500; strip any server path from the message before returning it. + throw new InvalidUploadException(sanitize(e.getMessage(), target)); + } + if (!registered) { + throw new TableAlreadyExistsException(name); + } + + // Registration succeeded; nothing below may throw, so the backing file must survive. + keep = true; + return new UploadResponse(name, toDto(catalog.columnsOf(name)), rowCount); + } finally { + if (!keep) { + Files.deleteIfExists(target); + } + } + } + + private void validateName(String name) { + if (name == null || !VALID_NAME.matcher(name).matches()) { + throw new IllegalArgumentException( + "Invalid table name '" + name + "'; must match [A-Za-z0-9_]{1,64}."); + } + } + + /** Builds a work-dir-relative target and proves it cannot escape that directory. */ + private Path resolveSafeTarget(String name) { + Path base = Path.of(properties.workDir()).toAbsolutePath().normalize(); + Path target = base.resolve(name + "-" + UUID.randomUUID() + ".csv").normalize(); + if (!target.startsWith(base)) { + throw new IllegalArgumentException("Resolved upload path escapes the work directory."); + } + return target; + } + + /** + * Streams the request body to {@code target}, aborting as soon as the byte count exceeds + * {@code maxBytes}, so an oversized upload is never fully buffered. The partial file is + * cleaned up by the caller's finally block. + */ + private static void copyWithCap(InputStream in, Path target, long maxBytes) throws IOException { + long total = 0; + byte[] buffer = new byte[8192]; + try (OutputStream out = Files.newOutputStream( + target, StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE)) { + int read; + while ((read = in.read(buffer)) != -1) { + total += read; + if (total > maxBytes) { + throw new PayloadTooLargeException(maxBytes); + } + out.write(buffer, 0, read); + } + } + } + + private static long countDataRows(Path csv) throws IOException { + try (Stream lines = Files.lines(csv)) { + return Math.max(0, lines.count() - 1); // drop the header row + } + } + + private static String sanitize(String message, Path target) { + if (message == null) { + return "Malformed CSV upload."; + } + // target is already absolute (resolveSafeTarget normalises against an absolute base), + // so target.toString() is the exact path the engine embeds in its messages. + return message.replace(target.toString(), ""); + } + + private static List toDto(List columns) { + List dtos = new ArrayList<>(columns.size()); + for (CatalogFacade.TableColumn c : columns) { + dtos.add(new TableColumnDto(c.name(), c.type() == null ? null : c.type().name())); + } + return dtos; + } +} diff --git a/server/src/main/java/com/github/jinba1/cuckoodb/server/web/TableLimitExceededException.java b/server/src/main/java/com/github/jinba1/cuckoodb/server/web/TableLimitExceededException.java new file mode 100644 index 0000000..08ee979 --- /dev/null +++ b/server/src/main/java/com/github/jinba1/cuckoodb/server/web/TableLimitExceededException.java @@ -0,0 +1,12 @@ +package com.github.jinba1.cuckoodb.server.web; + +/** + * The process-wide registered-table cap is reached and there is no eviction. Maps to 507 + * (Insufficient Storage): a capacity condition, not transient, so unlike concurrency saturation + * it carries no {@code Retry-After}. + */ +public class TableLimitExceededException extends RuntimeException { + public TableLimitExceededException(int maxTables) { + super("Table capacity reached (" + maxTables + " tables); no further uploads accepted."); + } +} diff --git a/server/src/main/java/com/github/jinba1/cuckoodb/server/web/TableNotFoundException.java b/server/src/main/java/com/github/jinba1/cuckoodb/server/web/TableNotFoundException.java new file mode 100644 index 0000000..cff58df --- /dev/null +++ b/server/src/main/java/com/github/jinba1/cuckoodb/server/web/TableNotFoundException.java @@ -0,0 +1,11 @@ +package com.github.jinba1.cuckoodb.server.web; + +/** + * The table named in a {@code GET /tables/{name}} URL does not exist. Maps to 404: here the + * table IS the addressed resource (unlike a query body's unknown table, which is a 422). + */ +public class TableNotFoundException extends RuntimeException { + public TableNotFoundException(String name) { + super("Table '" + name + "' not found."); + } +} diff --git a/server/src/main/java/com/github/jinba1/cuckoodb/server/web/UploadDisabledException.java b/server/src/main/java/com/github/jinba1/cuckoodb/server/web/UploadDisabledException.java new file mode 100644 index 0000000..2d69839 --- /dev/null +++ b/server/src/main/java/com/github/jinba1/cuckoodb/server/web/UploadDisabledException.java @@ -0,0 +1,11 @@ +package com.github.jinba1.cuckoodb.server.web; + +/** + * Upload is disabled ({@code cuckoodb.upload.enabled=false}). Maps to 404, not 403: a disabled + * write surface is treated as not mounted, so its existence is not advertised before governance. + */ +public class UploadDisabledException extends RuntimeException { + public UploadDisabledException() { + super("No such resource."); + } +} diff --git a/server/src/main/java/com/github/jinba1/cuckoodb/server/web/dto/ColumnDto.java b/server/src/main/java/com/github/jinba1/cuckoodb/server/web/dto/ColumnDto.java new file mode 100644 index 0000000..ff81730 --- /dev/null +++ b/server/src/main/java/com/github/jinba1/cuckoodb/server/web/dto/ColumnDto.java @@ -0,0 +1,16 @@ +package com.github.jinba1.cuckoodb.server.web.dto; + +import com.fasterxml.jackson.annotation.JsonInclude; + +/** + * One result column's metadata. {@code qualifiedName} disambiguates duplicate bare names from a + * join {@code SELECT *} (e.g. {@code student.a} vs {@code enrolled.a}); {@code type} is the + * best-effort inferred type and is null for an empty result. Both are omitted from JSON when null. + * + * @param name the bare header name (not unique across a join result) + * @param qualifiedName the dotted schema origin, or null when there is none + * @param type {@code INT} / {@code STRING}, or null for an empty result + */ +@JsonInclude(JsonInclude.Include.NON_NULL) +public record ColumnDto(String name, String qualifiedName, String type) { +} diff --git a/server/src/main/java/com/github/jinba1/cuckoodb/server/web/dto/ErrorResponse.java b/server/src/main/java/com/github/jinba1/cuckoodb/server/web/dto/ErrorResponse.java new file mode 100644 index 0000000..d3d72d1 --- /dev/null +++ b/server/src/main/java/com/github/jinba1/cuckoodb/server/web/dto/ErrorResponse.java @@ -0,0 +1,26 @@ +package com.github.jinba1.cuckoodb.server.web.dto; + +import com.fasterxml.jackson.annotation.JsonInclude; + +/** + * Uniform error body. {@code errorCode} is a stable machine-readable category (an engine + * {@code ErrorCode} name or a server-side code); {@code message} is agent-legible for 4xx and + * generic for 5xx. {@code errorId} is set only for 5xx, correlating the generic client message + * with the full detail logged server-side (raw engine messages can embed filesystem paths and + * are never serialized). + * + * @param errorCode stable failure category + * @param message human/agent readable explanation (generic for 5xx) + * @param errorId server-log correlation id for 5xx; null (omitted) otherwise + */ +@JsonInclude(JsonInclude.Include.NON_NULL) +public record ErrorResponse(String errorCode, String message, String errorId) { + + public static ErrorResponse of(String errorCode, String message) { + return new ErrorResponse(errorCode, message, null); + } + + public static ErrorResponse withId(String errorCode, String message, String errorId) { + return new ErrorResponse(errorCode, message, errorId); + } +} diff --git a/server/src/main/java/com/github/jinba1/cuckoodb/server/web/dto/QueryRequest.java b/server/src/main/java/com/github/jinba1/cuckoodb/server/web/dto/QueryRequest.java new file mode 100644 index 0000000..ab16e5f --- /dev/null +++ b/server/src/main/java/com/github/jinba1/cuckoodb/server/web/dto/QueryRequest.java @@ -0,0 +1,11 @@ +package com.github.jinba1.cuckoodb.server.web.dto; + +/** + * Request body for {@code POST /queries}. + * + * @param sql the SQL text to plan and execute (EXPLAIN-prefix allowed); required + * @param maxTuples optional client tuple budget; clamped down to the server cap, defaulted if null + * @param timeoutMs optional client time budget in ms; clamped down to the server cap, defaulted if null + */ +public record QueryRequest(String sql, Long maxTuples, Long timeoutMs) { +} diff --git a/server/src/main/java/com/github/jinba1/cuckoodb/server/web/dto/QueryResponse.java b/server/src/main/java/com/github/jinba1/cuckoodb/server/web/dto/QueryResponse.java new file mode 100644 index 0000000..47d6c45 --- /dev/null +++ b/server/src/main/java/com/github/jinba1/cuckoodb/server/web/dto/QueryResponse.java @@ -0,0 +1,73 @@ +package com.github.jinba1.cuckoodb.server.web.dto; + +import com.fasterxml.jackson.annotation.JsonInclude; +import com.github.jinba1.cuckoodb.ColumnMeta; +import com.github.jinba1.cuckoodb.IntValue; +import com.github.jinba1.cuckoodb.QueryResultSet; +import com.github.jinba1.cuckoodb.StringValue; +import com.github.jinba1.cuckoodb.Value; + +import java.util.ArrayList; +import java.util.List; + +/** + * Response body for {@code POST /queries}. Rows are positional arrays aligned with + * {@code columns} by index — chosen over row-objects because a join {@code SELECT *} can emit + * duplicate bare names that object keys would collapse. An EXPLAIN request populates only + * {@code explain}; an executed query leaves {@code explain} null. Null fields are omitted from + * JSON, so the two shapes are unambiguous on the wire. + * + * @param columns per-column metadata (null for EXPLAIN) + * @param rows positional value arrays (null for EXPLAIN) + * @param rowCount number of rows returned (null for EXPLAIN) + * @param truncated whether a LIMIT cut the result short (null for EXPLAIN) + * @param hint how to refine when truncated; null otherwise + * @param explain the rendered plan text for an EXPLAIN request; null otherwise + */ +@JsonInclude(JsonInclude.Include.NON_NULL) +public record QueryResponse( + List columns, + List> rows, + Long rowCount, + Boolean truncated, + String hint, + String explain) { + + /** Maps an executed result set, converting typed values to JSON-native ints/strings. */ + public static QueryResponse fromResultSet(QueryResultSet rs) { + List columns = new ArrayList<>(rs.columns().size()); + for (ColumnMeta c : rs.columns()) { + columns.add(new ColumnDto(c.name(), c.qualifiedName(), + c.type() == null ? null : c.type().name())); + } + List> rows = new ArrayList<>(rs.rows().size()); + for (List row : rs.rows()) { + List out = new ArrayList<>(row.size()); + for (Value v : row) { + out.add(toJson(v)); + } + rows.add(out); + } + return new QueryResponse(columns, rows, (long) rs.rows().size(), + rs.truncated(), rs.hint(), null); + } + + /** Wraps EXPLAIN plan text; columns/rows/rowCount/truncated stay null (no execution). */ + public static QueryResponse fromExplain(String explainText) { + return new QueryResponse(null, null, null, null, null, explainText); + } + + /** + * An {@code int} value serializes as a JSON number, a string as a JSON string — the typed + * distinction is intentional and deliberately lossy (an INT-typed {@code "007"} becomes 7). + */ + private static Object toJson(Value v) { + if (v instanceof IntValue iv) { + return iv.v(); + } + if (v instanceof StringValue sv) { + return sv.v(); + } + throw new IllegalStateException("Unknown value type: " + v.getClass().getName()); + } +} diff --git a/server/src/main/java/com/github/jinba1/cuckoodb/server/web/dto/TableColumnDto.java b/server/src/main/java/com/github/jinba1/cuckoodb/server/web/dto/TableColumnDto.java new file mode 100644 index 0000000..3107ac8 --- /dev/null +++ b/server/src/main/java/com/github/jinba1/cuckoodb/server/web/dto/TableColumnDto.java @@ -0,0 +1,10 @@ +package com.github.jinba1.cuckoodb.server.web.dto; + +/** + * One column of a base table's static, catalog-authoritative schema. + * + * @param name the bare column name + * @param type the catalog-inferred type ({@code INT} / {@code STRING}) + */ +public record TableColumnDto(String name, String type) { +} diff --git a/server/src/main/java/com/github/jinba1/cuckoodb/server/web/dto/TableSchemaResponse.java b/server/src/main/java/com/github/jinba1/cuckoodb/server/web/dto/TableSchemaResponse.java new file mode 100644 index 0000000..b7823d1 --- /dev/null +++ b/server/src/main/java/com/github/jinba1/cuckoodb/server/web/dto/TableSchemaResponse.java @@ -0,0 +1,13 @@ +package com.github.jinba1.cuckoodb.server.web.dto; + +import java.util.List; + +/** + * Response body for {@code GET /tables/{name}}: a table's static typed schema. Types come from + * the catalog, not from a query result, so schema discovery never depends on a non-empty result. + * + * @param name the table name + * @param columns the columns in column order + */ +public record TableSchemaResponse(String name, List columns) { +} diff --git a/server/src/main/java/com/github/jinba1/cuckoodb/server/web/dto/UploadResponse.java b/server/src/main/java/com/github/jinba1/cuckoodb/server/web/dto/UploadResponse.java new file mode 100644 index 0000000..20db15b --- /dev/null +++ b/server/src/main/java/com/github/jinba1/cuckoodb/server/web/dto/UploadResponse.java @@ -0,0 +1,13 @@ +package com.github.jinba1.cuckoodb.server.web.dto; + +import java.util.List; + +/** + * Response body for a successful {@code POST /tables/{name}} (201). + * + * @param name the registered table name + * @param columns the inferred typed schema, in column order + * @param rowCount the number of data rows ingested + */ +public record UploadResponse(String name, List columns, long rowCount) { +} diff --git a/server/src/main/resources/application.properties b/server/src/main/resources/application.properties new file mode 100644 index 0000000..f93ed16 --- /dev/null +++ b/server/src/main/resources/application.properties @@ -0,0 +1,36 @@ +# cuckooDB REST gateway configuration. Every value here is a conservative default; override +# per deployment via environment variables or an external application.properties. + +# --- Catalog --- +# Directory whose data/ subdir is loaded into the catalog at startup. Empty = uploads only. +cuckoodb.data-dir= +# Where uploaded CSVs are persisted for the process lifetime. MUST NOT be an auto-deleted temp +# dir: joins re-open table files on every scan, so the backing file must survive the request. +cuckoodb.work-dir=${java.io.tmpdir}/cuckoodb-work + +# --- Query budgets (always attached; the engine has no default = unlimited) --- +cuckoodb.query.max-tuples-default=100000 +cuckoodb.query.max-tuples-cap=1000000 +cuckoodb.query.timeout-ms-default=5000 +cuckoodb.query.timeout-ms-cap=30000 + +# --- Concurrency (CPU-bound engine) --- +# Blank/absent => Runtime.availableProcessors(). +cuckoodb.max-concurrent-queries= + +# --- Upload (write surface; closed by default until a later governance phase enables it) --- +cuckoodb.upload.enabled=false +cuckoodb.upload.max-bytes=10485760 +cuckoodb.upload.max-tables=100 + +# --- Container limits cohere with the budgets above --- +# The real upload size cap is enforced by the controller WHILE streaming the body (-> 413). +# These are only coarse container backstops. NOTE: spring.servlet.multipart.* does NOT apply to +# this endpoint, which consumes raw text/csv via getInputStream(), not multipart/form-data. +server.tomcat.max-swallow-size=50MB +# Keep the connection timeout above the time-budget cap so a slow query surfaces as a clean +# 504/429 rather than a container-level reset. +server.tomcat.connection-timeout=35s + +# OpenAPI / Swagger UI +springdoc.swagger-ui.path=/swagger-ui.html diff --git a/server/src/test/java/com/github/jinba1/cuckoodb/server/query/BudgetPolicyTest.java b/server/src/test/java/com/github/jinba1/cuckoodb/server/query/BudgetPolicyTest.java new file mode 100644 index 0000000..3f04332 --- /dev/null +++ b/server/src/test/java/com/github/jinba1/cuckoodb/server/query/BudgetPolicyTest.java @@ -0,0 +1,48 @@ +package com.github.jinba1.cuckoodb.server.query; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import com.github.jinba1.cuckoodb.server.config.CuckooDbProperties; + +import org.junit.jupiter.api.Test; + +/** + * BudgetPolicy must never yield a null-valued budget (the engine's "null = unlimited" path is + * then unreachable through the server) and the configured hard cap must always win over a + * larger client request. + */ +class BudgetPolicyTest { + + /** Properties with the documented defaults: tuples 100k/1M, time 5s/30s. */ + private BudgetPolicy policy() { + return new BudgetPolicy(new CuckooDbProperties(null, null, null, null, null)); + } + + @Test + void nullRequestTakesDefaults() { + BudgetPolicy.ClampedBudget budget = policy().clamp(null, null); + assertEquals(100_000L, budget.maxTuples()); + assertEquals(5_000L, budget.timeoutMs()); + } + + @Test + void requestAboveCapIsClampedDownToCap() { + BudgetPolicy.ClampedBudget budget = policy().clamp(10_000_000L, 999_999L); + assertEquals(1_000_000L, budget.maxTuples(), "tuple cap always wins"); + assertEquals(30_000L, budget.timeoutMs(), "time cap always wins"); + } + + @Test + void requestWithinCapPassesThrough() { + BudgetPolicy.ClampedBudget budget = policy().clamp(500L, 1_000L); + assertEquals(500L, budget.maxTuples()); + assertEquals(1_000L, budget.timeoutMs()); + } + + @Test + void nonPositiveRequestIsRejected() { + assertThrows(IllegalArgumentException.class, () -> policy().clamp(0L, 1_000L)); + assertThrows(IllegalArgumentException.class, () -> policy().clamp(100L, -1L)); + } +} diff --git a/server/src/test/java/com/github/jinba1/cuckoodb/server/query/ConcurrencyLimiterTest.java b/server/src/test/java/com/github/jinba1/cuckoodb/server/query/ConcurrencyLimiterTest.java new file mode 100644 index 0000000..b8b6893 --- /dev/null +++ b/server/src/test/java/com/github/jinba1/cuckoodb/server/query/ConcurrencyLimiterTest.java @@ -0,0 +1,38 @@ +package com.github.jinba1.cuckoodb.server.query; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import com.github.jinba1.cuckoodb.server.config.CuckooDbProperties; + +import org.junit.jupiter.api.Test; + +class ConcurrencyLimiterTest { + + private ConcurrencyLimiter withPermits(int n) { + return new ConcurrencyLimiter(new CuckooDbProperties(null, null, n, null, null)); + } + + @Test + void runsWorkAndReleasesPermit() { + ConcurrencyLimiter limiter = withPermits(1); + assertEquals("ok", limiter.withPermit(() -> "ok")); + assertEquals(1, limiter.availablePermits(), "permit released after work"); + } + + @Test + void saturationFailsFast() { + // One permit, already held by the outer call: the nested acquire must fail, not block. + ConcurrencyLimiter limiter = withPermits(1); + assertThrows(ConcurrencyLimitExceededException.class, + () -> limiter.withPermit(() -> limiter.withPermit(() -> "inner"))); + } + + @Test + void releasesPermitEvenWhenWorkThrows() { + ConcurrencyLimiter limiter = withPermits(1); + assertThrows(RuntimeException.class, + () -> limiter.withPermit(() -> { throw new RuntimeException("boom"); })); + assertEquals(1, limiter.availablePermits(), "permit released on failure too"); + } +} diff --git a/server/src/test/java/com/github/jinba1/cuckoodb/server/web/BudgetIntegrationTest.java b/server/src/test/java/com/github/jinba1/cuckoodb/server/web/BudgetIntegrationTest.java new file mode 100644 index 0000000..ad24d12 --- /dev/null +++ b/server/src/test/java/com/github/jinba1/cuckoodb/server/web/BudgetIntegrationTest.java @@ -0,0 +1,111 @@ +package com.github.jinba1.cuckoodb.server.web; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import com.fasterxml.jackson.databind.ObjectMapper; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Comparator; + +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.web.client.TestRestTemplate; +import org.springframework.http.HttpEntity; +import org.springframework.http.HttpHeaders; +import org.springframework.http.MediaType; +import org.springframework.http.ResponseEntity; +import org.springframework.test.annotation.DirtiesContext; +import org.springframework.test.context.DynamicPropertyRegistry; +import org.springframework.test.context.DynamicPropertySource; + +/** + * Proves the fail-closed budget guarantee: the server always attaches a budget, so even + * a client requesting an enormous tuple limit is clamped to the cap and cannot run unbounded; + * a query that omits any budget still trips the configured default; and EXPLAIN consumes no + * budget at all. The whole context runs with a deliberately tiny cap (2 tuples). + */ +@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) +@DirtiesContext(classMode = DirtiesContext.ClassMode.AFTER_CLASS) +class BudgetIntegrationTest { + + private static final ObjectMapper JSON = new ObjectMapper(); + + @Autowired + private TestRestTemplate rest; + + private static Path dataDir; + private static Path workDir; + + @DynamicPropertySource + static void properties(DynamicPropertyRegistry registry) throws IOException { + dataDir = Files.createTempDirectory("cuckoo-data"); + Path data = Files.createDirectories(dataDir.resolve("data")); + // 4 rows: any full scan charges 4 tuples, exceeding the 2-tuple cap below. + Files.writeString(data.resolve("People.csv"), "id\n1\n2\n3\n4\n"); + workDir = Files.createTempDirectory("cuckoo-work"); + + registry.add("cuckoodb.data-dir", () -> dataDir.toString()); + registry.add("cuckoodb.work-dir", () -> workDir.toString()); + registry.add("cuckoodb.query.max-tuples-default", () -> "2"); + registry.add("cuckoodb.query.max-tuples-cap", () -> "2"); + } + + @AfterAll + static void cleanup() throws IOException { + deleteRecursively(dataDir); + deleteRecursively(workDir); + } + + @Test + void omittedBudgetStillEnforcesTheDefault() throws Exception { + // No maxTuples in the request: the server must still attach the default budget (2), + // which a 4-row scan exceeds. A 200 here would mean the unbounded path was reachable. + ResponseEntity resp = postQuery("{\"sql\":\"SELECT * FROM People\"}"); + assertEquals(429, resp.getStatusCode().value(), resp.getBody()); + assertEquals("BUDGET_EXCEEDED", JSON.readTree(resp.getBody()).get("errorCode").asText()); + assertTrue(resp.getHeaders().containsKey("Retry-After")); + } + + @Test + void hugeClientBudgetIsClampedToCapAndStillTrips() throws Exception { + // The client cannot buy its way past the cap: 9,999,999 clamps to 2, the scan still trips. + ResponseEntity resp = postQuery( + "{\"sql\":\"SELECT * FROM People\",\"maxTuples\":9999999}"); + assertEquals(429, resp.getStatusCode().value(), resp.getBody()); + assertEquals("BUDGET_EXCEEDED", JSON.readTree(resp.getBody()).get("errorCode").asText()); + } + + @Test + void explainConsumesNoBudget() throws Exception { + // EXPLAIN performs no execution, so the tiny budget never bites: a plan comes back 200. + ResponseEntity resp = postQuery("{\"sql\":\"EXPLAIN SELECT * FROM People\"}"); + assertEquals(200, resp.getStatusCode().value(), resp.getBody()); + assertTrue(JSON.readTree(resp.getBody()).get("explain").asText().contains("Plan")); + } + + private ResponseEntity postQuery(String json) { + HttpHeaders headers = new HttpHeaders(); + headers.setContentType(MediaType.APPLICATION_JSON); + return rest.postForEntity("/queries", new HttpEntity<>(json, headers), String.class); + } + + private static void deleteRecursively(Path root) throws IOException { + if (root == null || !Files.exists(root)) { + return; + } + try (var paths = Files.walk(root)) { + paths.sorted(Comparator.reverseOrder()).forEach(p -> { + try { + Files.deleteIfExists(p); + } catch (IOException ignored) { + // best-effort temp cleanup + } + }); + } + } +} diff --git a/server/src/test/java/com/github/jinba1/cuckoodb/server/web/QueryApiIntegrationTest.java b/server/src/test/java/com/github/jinba1/cuckoodb/server/web/QueryApiIntegrationTest.java new file mode 100644 index 0000000..eea7079 --- /dev/null +++ b/server/src/test/java/com/github/jinba1/cuckoodb/server/web/QueryApiIntegrationTest.java @@ -0,0 +1,185 @@ +package com.github.jinba1.cuckoodb.server.web; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Comparator; +import java.util.List; +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; + +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.web.client.TestRestTemplate; +import org.springframework.http.HttpEntity; +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpMethod; +import org.springframework.http.MediaType; +import org.springframework.http.ResponseEntity; +import org.springframework.test.annotation.DirtiesContext; +import org.springframework.test.context.DynamicPropertyRegistry; +import org.springframework.test.context.DynamicPropertySource; + +/** + * End-to-end gateway behaviour over a real HTTP port and a temp-dir catalog: the sync query + * round-trip, EXPLAIN, 5xx path-scrubbing, the upload-disabled default, and concurrent queries. + * Budget bounding has its own class ({@link BudgetIntegrationTest}); upload has + * {@link UploadApiIntegrationTest}. Catalog isolation is by fresh context ({@code @DirtiesContext}), + * never {@code resetDBCatalog} — the engine singleton is owned by the app's CatalogInitializer. + */ +@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) +@DirtiesContext(classMode = DirtiesContext.ClassMode.AFTER_CLASS) +class QueryApiIntegrationTest { + + private static final ObjectMapper JSON = new ObjectMapper(); + + @Autowired + private TestRestTemplate rest; + + private static Path dataDir; + private static Path workDir; + + @DynamicPropertySource + static void properties(DynamicPropertyRegistry registry) throws IOException { + dataDir = Files.createTempDirectory("cuckoo-data"); + Path data = Files.createDirectories(dataDir.resolve("data")); + Files.writeString(data.resolve("People.csv"), "id,name\n1,alice\n2,bob\n3,carol\n"); + // A throwaway table whose backing file the 5xx test deletes to force a query-path error. + Files.writeString(data.resolve("Doomed.csv"), "x,y\n1,2\n"); + workDir = Files.createTempDirectory("cuckoo-work"); + + registry.add("cuckoodb.data-dir", () -> dataDir.toString()); + registry.add("cuckoodb.work-dir", () -> workDir.toString()); + registry.add("cuckoodb.upload.enabled", () -> "false"); + // Pin the concurrency ceiling wide enough for the 16-thread test below; otherwise it + // defaults to availableProcessors() and the excess threads get 429 on low-CPU CI runners. + // The 429 saturation path itself is covered by ConcurrencyLimiterTest + QueryControllerTest. + registry.add("cuckoodb.max-concurrent-queries", () -> "32"); + } + + @AfterAll + static void cleanup() throws IOException { + deleteRecursively(dataDir); + deleteRecursively(workDir); + } + + @Test + void syncQueryRoundTripReturnsTypedColumnArrays() throws Exception { + JsonNode body = postQuery("{\"sql\":\"SELECT * FROM People\"}", 200); + assertEquals(3, body.get("rowCount").asInt()); + assertFalse(body.get("truncated").asBoolean()); + assertEquals("id", body.get("columns").get(0).get("name").asText()); + assertEquals("INT", body.get("columns").get(0).get("type").asText()); + assertEquals("STRING", body.get("columns").get(1).get("type").asText()); + assertEquals(1, body.get("rows").get(0).get(0).asInt()); + assertEquals("alice", body.get("rows").get(0).get(1).asText()); + } + + @Test + void limitMarksResultTruncated() throws Exception { + JsonNode body = postQuery("{\"sql\":\"SELECT * FROM People LIMIT 1\"}", 200); + assertEquals(1, body.get("rowCount").asInt()); + assertTrue(body.get("truncated").asBoolean()); + assertNotNull(body.get("hint")); + } + + @Test + void explainReturnsPlanWithNoRowsAndNoRowCount() throws Exception { + JsonNode body = postQuery("{\"sql\":\"EXPLAIN SELECT * FROM People\"}", 200); + assertTrue(body.get("explain").asText().contains("Plan (as written)")); + assertFalse(body.has("rows"), "EXPLAIN performs no execution, so rows is absent"); + assertFalse(body.has("rowCount")); + } + + @Test + void unknownTableInQueryBodyIs422() throws Exception { + JsonNode body = postQuery("{\"sql\":\"SELECT * FROM Nope\"}", 422); + assertEquals("UNKNOWN_TABLE", body.get("errorCode").asText()); + } + + @Test + void queryPathDataErrorReturns500WithNoFilesystemPath() throws Exception { + // Delete a table's backing file after startup: the scan now fails with a DATA_ERROR whose + // raw message embeds the absolute file path. The 500 body must not leak it. + Files.deleteIfExists(dataDir.resolve("data").resolve("Doomed.csv")); + + ResponseEntity resp = exchange("/queries", HttpMethod.POST, + MediaType.APPLICATION_JSON, "{\"sql\":\"SELECT * FROM Doomed\"}"); + assertEquals(500, resp.getStatusCode().value()); + String raw = resp.getBody(); + assertNotNull(raw); + assertFalse(raw.contains(dataDir.toString()), "5xx body leaked a filesystem path: " + raw); + JsonNode body = JSON.readTree(raw); + assertNotNull(body.get("errorId"), "5xx carries a correlation id"); + assertEquals("Internal server error.", body.get("message").asText()); + } + + @Test + void uploadDisabledByDefaultReturns404() { + ResponseEntity resp = exchange("/tables/foo", HttpMethod.POST, + MediaType.parseMediaType("text/csv"), "a,b\n1,2\n"); + assertEquals(404, resp.getStatusCode().value()); + } + + @Test + void concurrentQueriesAllSucceed() throws Exception { + int threads = 16; + ExecutorService pool = Executors.newFixedThreadPool(threads); + try { + List> tasks = new ArrayList<>(); + for (int i = 0; i < threads; i++) { + tasks.add(() -> postQuery("{\"sql\":\"SELECT People.id FROM People WHERE People.id > 1\"}", 200)); + } + List> results = pool.invokeAll(tasks); + for (Future f : results) { + assertEquals(2, f.get().get("rowCount").asInt(), + "every concurrent query sees a consistent, isolated result"); + } + } finally { + pool.shutdownNow(); + } + } + + private JsonNode postQuery(String json, int expectedStatus) throws IOException { + ResponseEntity resp = exchange("/queries", HttpMethod.POST, + MediaType.APPLICATION_JSON, json); + assertEquals(expectedStatus, resp.getStatusCode().value(), + "unexpected status; body=" + resp.getBody()); + return JSON.readTree(resp.getBody()); + } + + private ResponseEntity exchange(String path, HttpMethod method, + MediaType contentType, String body) { + HttpHeaders headers = new HttpHeaders(); + headers.setContentType(contentType); + return rest.exchange(path, method, new HttpEntity<>(body, headers), String.class); + } + + private static void deleteRecursively(Path root) throws IOException { + if (root == null || !Files.exists(root)) { + return; + } + try (var paths = Files.walk(root)) { + paths.sorted(Comparator.reverseOrder()).forEach(p -> { + try { + Files.deleteIfExists(p); + } catch (IOException ignored) { + // best-effort temp cleanup + } + }); + } + } +} diff --git a/server/src/test/java/com/github/jinba1/cuckoodb/server/web/QueryControllerTest.java b/server/src/test/java/com/github/jinba1/cuckoodb/server/web/QueryControllerTest.java new file mode 100644 index 0000000..4afb050 --- /dev/null +++ b/server/src/test/java/com/github/jinba1/cuckoodb/server/web/QueryControllerTest.java @@ -0,0 +1,216 @@ +package com.github.jinba1.cuckoodb.server.web; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.isNull; +import static org.mockito.Mockito.when; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import com.github.jinba1.cuckoodb.BudgetKind; +import com.github.jinba1.cuckoodb.ColumnMeta; +import com.github.jinba1.cuckoodb.ColumnType; +import com.github.jinba1.cuckoodb.ErrorCode; +import com.github.jinba1.cuckoodb.IntValue; +import com.github.jinba1.cuckoodb.QueryBudgetExceededException; +import com.github.jinba1.cuckoodb.QueryExecutionException; +import com.github.jinba1.cuckoodb.QueryResultSet; +import com.github.jinba1.cuckoodb.StringValue; +import com.github.jinba1.cuckoodb.Value; +import com.github.jinba1.cuckoodb.server.query.ApiPrincipalResolver; +import com.github.jinba1.cuckoodb.server.query.ConcurrencyLimitExceededException; +import com.github.jinba1.cuckoodb.server.query.QueryService; +import com.github.jinba1.cuckoodb.server.query.QueryServiceResult; + +import java.util.List; + +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.WebMvcTest; +import org.springframework.http.MediaType; +import org.springframework.test.context.bean.override.mockito.MockitoBean; +import org.springframework.test.web.servlet.MockMvc; + +/** + * Controller slice for {@code POST /queries}: response JSON shape and the full ErrorCode → HTTP + * mapping, driven by a mocked {@link QueryService} so each branch is exercised in isolation. + */ +@WebMvcTest(QueryController.class) +class QueryControllerTest { + + @Autowired + private MockMvc mvc; + + @MockitoBean + private QueryService queryService; + + @MockitoBean + private ApiPrincipalResolver principalResolver; + + @BeforeEach + void anonymousPrincipal() { + when(principalResolver.resolve(any())).thenReturn("anonymous"); + } + + private static String body(String sql) { + return "{\"sql\":\"" + sql + "\"}"; + } + + private org.springframework.test.web.servlet.ResultActions postQuery(String sql) throws Exception { + return mvc.perform(post("/queries") + .contentType(MediaType.APPLICATION_JSON) + .content(body(sql))); + } + + private void serviceThrows(QueryExecutionException ex) { + when(queryService.execute(any(), any(), any(), any())).thenThrow(ex); + } + + @Test + void executedQueryReturns200WithColumnArraysAndRowCount() throws Exception { + QueryResultSet rs = new QueryResultSet( + List.of(new ColumnMeta("a", "student.a", ColumnType.INT), + new ColumnMeta("name", "student.name", ColumnType.STRING)), + List.of(List.of(new IntValue(1), new StringValue("alice"))), + false, null); + when(queryService.execute(eq("SELECT * FROM Student"), isNull(), isNull(), eq("anonymous"))) + .thenReturn(QueryServiceResult.of(rs)); + + postQuery("SELECT * FROM Student") + .andExpect(status().isOk()) + .andExpect(jsonPath("$.rowCount").value(1)) + .andExpect(jsonPath("$.truncated").value(false)) + .andExpect(jsonPath("$.columns[0].name").value("a")) + .andExpect(jsonPath("$.columns[0].qualifiedName").value("student.a")) + .andExpect(jsonPath("$.columns[0].type").value("INT")) + .andExpect(jsonPath("$.rows[0][0]").value(1)) + .andExpect(jsonPath("$.rows[0][1]").value("alice")) + .andExpect(jsonPath("$.explain").doesNotExist()); + } + + @Test + void explainReturns200WithExplainSetAndNoRows() throws Exception { + when(queryService.execute(any(), any(), any(), any())) + .thenReturn(QueryServiceResult.explain("=== Plan (as written) ===\nScan[Student]")); + + postQuery("EXPLAIN SELECT * FROM Student") + .andExpect(status().isOk()) + .andExpect(jsonPath("$.explain").value(org.hamcrest.Matchers.containsString("Plan"))) + .andExpect(jsonPath("$.rows").doesNotExist()) + .andExpect(jsonPath("$.columns").doesNotExist()) + .andExpect(jsonPath("$.rowCount").doesNotExist()); + } + + @Test + void parseErrorMapsTo400() throws Exception { + serviceThrows(new QueryExecutionException(ErrorCode.PARSE_ERROR, "SQL syntax error: bad")); + postQuery("SELECT FROM") + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.errorCode").value("PARSE_ERROR")) + .andExpect(jsonPath("$.message").value(org.hamcrest.Matchers.containsString("syntax"))); + } + + @Test + void unsupportedSqlMapsTo422() throws Exception { + serviceThrows(new QueryExecutionException(ErrorCode.UNSUPPORTED_SQL, "Only SELECT")); + postQuery("DELETE FROM Student").andExpect(status().isUnprocessableEntity()) + .andExpect(jsonPath("$.errorCode").value("UNSUPPORTED_SQL")); + } + + @Test + void unknownTableOnQueryPathMapsTo422() throws Exception { + serviceThrows(new QueryExecutionException(ErrorCode.UNKNOWN_TABLE, + "Table 'Nope' not found. Available tables: Student.")); + postQuery("SELECT * FROM Nope").andExpect(status().isUnprocessableEntity()) + .andExpect(jsonPath("$.errorCode").value("UNKNOWN_TABLE")) + .andExpect(jsonPath("$.message").value(org.hamcrest.Matchers.containsString("Available tables"))); + } + + @Test + void unknownColumnMapsTo422() throws Exception { + serviceThrows(new QueryExecutionException(ErrorCode.UNKNOWN_COLUMN, + "Column 'z' not found in Student")); + postQuery("SELECT Student.z FROM Student") + .andExpect(status().isUnprocessableEntity()) + .andExpect(jsonPath("$.errorCode").value("UNKNOWN_COLUMN")); + } + + @Test + void typeMismatchMapsTo422() throws Exception { + serviceThrows(new QueryExecutionException(ErrorCode.TYPE_MISMATCH, "cannot compare")); + postQuery("SELECT * FROM Student WHERE Student.a = 'x'") + .andExpect(status().isUnprocessableEntity()) + .andExpect(jsonPath("$.errorCode").value("TYPE_MISMATCH")); + } + + @Test + void tupleBudgetMapsTo429WithRetryAfter() throws Exception { + serviceThrows(new QueryBudgetExceededException(BudgetKind.TUPLES, "Tuple budget exceeded")); + postQuery("SELECT * FROM Big") + .andExpect(status().isTooManyRequests()) + .andExpect(header().exists("Retry-After")) + .andExpect(jsonPath("$.errorCode").value("BUDGET_EXCEEDED")); + } + + @Test + void timeBudgetMapsTo504() throws Exception { + serviceThrows(new QueryBudgetExceededException(BudgetKind.TIME, "Time budget exceeded")); + postQuery("SELECT * FROM Slow") + .andExpect(status().isGatewayTimeout()) + .andExpect(jsonPath("$.errorCode").value("BUDGET_EXCEEDED")); + } + + @Test + void concurrencySaturationMapsTo429WithRetryAfter() throws Exception { + when(queryService.execute(any(), any(), any(), any())) + .thenThrow(new ConcurrencyLimitExceededException("at limit")); + postQuery("SELECT * FROM Student") + .andExpect(status().isTooManyRequests()) + .andExpect(header().exists("Retry-After")) + .andExpect(jsonPath("$.errorCode").value("CONCURRENCY_LIMIT")); + } + + @Test + void queryPathDataErrorMapsTo500WithGenericScrubbedBody() throws Exception { + serviceThrows(new QueryExecutionException(ErrorCode.DATA_ERROR, + "Failed to open table 'x': /var/secret/path/data.csv broken")); + postQuery("SELECT * FROM X") + .andExpect(status().isInternalServerError()) + .andExpect(jsonPath("$.errorId").exists()) + .andExpect(jsonPath("$.message").value("Internal server error.")) + .andExpect(content().string(org.hamcrest.Matchers.not( + org.hamcrest.Matchers.containsString("/var/secret")))); + } + + @Test + void internalErrorMapsTo500Generic() throws Exception { + serviceThrows(new QueryExecutionException(ErrorCode.INTERNAL, "invariant broke at /home/x")); + postQuery("SELECT * FROM X") + .andExpect(status().isInternalServerError()) + .andExpect(jsonPath("$.errorId").exists()) + .andExpect(content().string(org.hamcrest.Matchers.not( + org.hamcrest.Matchers.containsString("/home/x")))); + } + + @Test + void nonPositiveBudgetMapsTo400() throws Exception { + when(queryService.execute(any(), anyLong(), any(), any())) + .thenThrow(new IllegalArgumentException("maxTuples must be a positive integer; got 0")); + mvc.perform(post("/queries").contentType(MediaType.APPLICATION_JSON) + .content("{\"sql\":\"SELECT * FROM Student\",\"maxTuples\":0}")) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.errorCode").value("BAD_REQUEST")); + } + + @Test + void malformedJsonBodyMapsTo400() throws Exception { + mvc.perform(post("/queries").contentType(MediaType.APPLICATION_JSON).content("{not json")) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.errorCode").value("BAD_REQUEST")); + } +} diff --git a/server/src/test/java/com/github/jinba1/cuckoodb/server/web/TableControllerTest.java b/server/src/test/java/com/github/jinba1/cuckoodb/server/web/TableControllerTest.java new file mode 100644 index 0000000..a3b27c1 --- /dev/null +++ b/server/src/test/java/com/github/jinba1/cuckoodb/server/web/TableControllerTest.java @@ -0,0 +1,87 @@ +package com.github.jinba1.cuckoodb.server.web; + +import static org.mockito.Mockito.when; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import com.github.jinba1.cuckoodb.ColumnType; +import com.github.jinba1.cuckoodb.server.catalog.CatalogFacade; + +import java.util.List; + +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest; +import org.springframework.test.context.bean.override.mockito.MockitoBean; +import org.springframework.test.web.servlet.MockMvc; + +/** + * Controller slice for the {@code /tables} endpoints. Covers the read surface, table-name + * validation, and the upload-disabled / wrong-content-type rejections; the full upload pipeline + * (streaming, size cap, 409, malformed CSV) is exercised end-to-end in the integration test. + * + *

{@code CuckooDbProperties} comes from the app's {@code @EnableConfigurationProperties} + * (bound from {@code application.properties}, where upload is disabled by default) — no extra + * bean is declared here, to avoid a duplicate-bean clash with that one. + */ +@WebMvcTest(TableController.class) +class TableControllerTest { + + @Autowired + private MockMvc mvc; + + @MockitoBean + private CatalogFacade catalog; + + @Test + void listReturnsSortedNames() throws Exception { + when(catalog.tableNames()).thenReturn(List.of("Course", "Student")); + mvc.perform(get("/tables")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$[0]").value("Course")) + .andExpect(jsonPath("$[1]").value("Student")); + } + + @Test + void describeReturnsStaticTypedSchema() throws Exception { + when(catalog.columnsOf("Student")).thenReturn(List.of( + new CatalogFacade.TableColumn("a", ColumnType.INT), + new CatalogFacade.TableColumn("name", ColumnType.STRING))); + mvc.perform(get("/tables/Student")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.name").value("Student")) + .andExpect(jsonPath("$.columns[0].name").value("a")) + .andExpect(jsonPath("$.columns[0].type").value("INT")) + .andExpect(jsonPath("$.columns[1].type").value("STRING")); + } + + @Test + void describeMissingTableReturns404() throws Exception { + when(catalog.columnsOf("Ghost")).thenReturn(null); + mvc.perform(get("/tables/Ghost")) + .andExpect(status().isNotFound()) + .andExpect(jsonPath("$.errorCode").value("UNKNOWN_TABLE")); + } + + @Test + void describeInvalidNameReturns400() throws Exception { + mvc.perform(get("/tables/bad-name")) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.errorCode").value("BAD_REQUEST")); + } + + @Test + void uploadWhenDisabledReturns404() throws Exception { + mvc.perform(post("/tables/foo").contentType("text/csv").content("a,b\n1,2\n")) + .andExpect(status().isNotFound()); + } + + @Test + void uploadWrongContentTypeReturns415() throws Exception { + mvc.perform(post("/tables/foo").contentType("text/plain").content("a,b\n1,2\n")) + .andExpect(status().isUnsupportedMediaType()) + .andExpect(jsonPath("$.errorCode").value("UNSUPPORTED_MEDIA_TYPE")); + } +} diff --git a/server/src/test/java/com/github/jinba1/cuckoodb/server/web/UploadApiIntegrationTest.java b/server/src/test/java/com/github/jinba1/cuckoodb/server/web/UploadApiIntegrationTest.java new file mode 100644 index 0000000..70797b5 --- /dev/null +++ b/server/src/test/java/com/github/jinba1/cuckoodb/server/web/UploadApiIntegrationTest.java @@ -0,0 +1,151 @@ +package com.github.jinba1.cuckoodb.server.web; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Comparator; + +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.web.client.TestRestTemplate; +import org.springframework.http.HttpEntity; +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpMethod; +import org.springframework.http.MediaType; +import org.springframework.http.ResponseEntity; +import org.springframework.test.annotation.DirtiesContext; +import org.springframework.test.context.DynamicPropertyRegistry; +import org.springframework.test.context.DynamicPropertySource; + +/** + * End-to-end upload pipeline with the feature enabled and a small byte cap: a successful + * upload-then-query, plus every rejection — oversize (413), wrong content-type (415), bad table + * name (400), duplicate name (409), and malformed CSV (400, path-scrubbed). + */ +@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) +@DirtiesContext(classMode = DirtiesContext.ClassMode.AFTER_CLASS) +class UploadApiIntegrationTest { + + private static final ObjectMapper JSON = new ObjectMapper(); + private static final MediaType TEXT_CSV = MediaType.parseMediaType("text/csv"); + + @Autowired + private TestRestTemplate rest; + + private static Path dataDir; + private static Path workDir; + + @DynamicPropertySource + static void properties(DynamicPropertyRegistry registry) throws IOException { + dataDir = Files.createTempDirectory("cuckoo-data"); + Files.createDirectories(dataDir.resolve("data")); // empty seed; tables arrive via upload + workDir = Files.createTempDirectory("cuckoo-work"); + + registry.add("cuckoodb.data-dir", () -> dataDir.toString()); + registry.add("cuckoodb.work-dir", () -> workDir.toString()); + registry.add("cuckoodb.upload.enabled", () -> "true"); + registry.add("cuckoodb.upload.max-bytes", () -> "64"); // tiny, to exercise the 413 path + } + + @AfterAll + static void cleanup() throws IOException { + deleteRecursively(dataDir); + deleteRecursively(workDir); + } + + @Test + void uploadThenQueryRoundTrips() throws Exception { + ResponseEntity upload = csv("/tables/Widgets", "sku,qty\nA,5\nB,7\n"); + assertEquals(201, upload.getStatusCode().value(), upload.getBody()); + JsonNode created = JSON.readTree(upload.getBody()); + assertEquals("Widgets", created.get("name").asText()); + assertEquals(2, created.get("rowCount").asInt()); + assertEquals("qty", created.get("columns").get(1).get("name").asText()); + assertEquals("INT", created.get("columns").get(1).get("type").asText()); + + // The freshly uploaded table is immediately queryable. + ResponseEntity query = exchange("/queries", HttpMethod.POST, + MediaType.APPLICATION_JSON, "{\"sql\":\"SELECT * FROM Widgets\"}"); + assertEquals(200, query.getStatusCode().value(), query.getBody()); + assertEquals(2, JSON.readTree(query.getBody()).get("rowCount").asInt()); + + // And it is listed and describable via the catalog endpoints. + ResponseEntity describe = rest.getForEntity("/tables/Widgets", String.class); + assertEquals(200, describe.getStatusCode().value()); + assertEquals("STRING", JSON.readTree(describe.getBody()) + .get("columns").get(0).get("type").asText(), "sku column is string-typed"); + } + + @Test + void oversizeUploadReturns413() { + String big = "n\n" + "1\n".repeat(40); // ~82 bytes > 64-byte cap + ResponseEntity resp = csv("/tables/Big", big); + assertEquals(413, resp.getStatusCode().value()); + } + + @Test + void wrongContentTypeReturns415() { + ResponseEntity resp = exchange("/tables/Json", HttpMethod.POST, + MediaType.APPLICATION_JSON, "sku,qty\nA,5\n"); + assertEquals(415, resp.getStatusCode().value()); + } + + @Test + void invalidTableNameReturns400() { + // A dotted name is a path-traversal shape; the strict charset rejects it before any path. + ResponseEntity resp = csv("/tables/a.b", "x\n1\n"); + assertEquals(400, resp.getStatusCode().value()); + } + + @Test + void duplicateUploadReturns409() { + assertEquals(201, csv("/tables/Dup", "x\n1\n").getStatusCode().value()); + assertEquals(409, csv("/tables/Dup", "x\n2\n").getStatusCode().value()); + } + + @Test + void malformedCsvReturns400WithNoFilesystemPath() throws Exception { + // Ragged row: header has 2 columns, the data row has 1 -> engine DATA_ERROR -> 400. + ResponseEntity resp = csv("/tables/Ragged", "a,b\n1\n"); + assertEquals(400, resp.getStatusCode().value()); + String raw = resp.getBody(); + assertNotNull(raw); + assertFalse(raw.contains(workDir.toString()), "upload 400 leaked a work-dir path: " + raw); + assertEquals("DATA_ERROR", JSON.readTree(raw).get("errorCode").asText()); + } + + private ResponseEntity csv(String path, String body) { + return exchange(path, HttpMethod.POST, TEXT_CSV, body); + } + + private ResponseEntity exchange(String path, HttpMethod method, + MediaType contentType, String body) { + HttpHeaders headers = new HttpHeaders(); + headers.setContentType(contentType); + return rest.exchange(path, method, new HttpEntity<>(body, headers), String.class); + } + + private static void deleteRecursively(Path root) throws IOException { + if (root == null || !Files.exists(root)) { + return; + } + try (var paths = Files.walk(root)) { + paths.sorted(Comparator.reverseOrder()).forEach(p -> { + try { + Files.deleteIfExists(p); + } catch (IOException ignored) { + // best-effort temp cleanup + } + }); + } + } +} diff --git a/server/src/test/java/com/github/jinba1/cuckoodb/server/web/UploadLimitIntegrationTest.java b/server/src/test/java/com/github/jinba1/cuckoodb/server/web/UploadLimitIntegrationTest.java new file mode 100644 index 0000000..970a7e5 --- /dev/null +++ b/server/src/test/java/com/github/jinba1/cuckoodb/server/web/UploadLimitIntegrationTest.java @@ -0,0 +1,92 @@ +package com.github.jinba1.cuckoodb.server.web; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; + +import com.fasterxml.jackson.databind.ObjectMapper; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Comparator; + +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.web.client.TestRestTemplate; +import org.springframework.http.HttpEntity; +import org.springframework.http.HttpHeaders; +import org.springframework.http.MediaType; +import org.springframework.http.ResponseEntity; +import org.springframework.test.annotation.DirtiesContext; +import org.springframework.test.context.DynamicPropertyRegistry; +import org.springframework.test.context.DynamicPropertySource; + +/** + * Proves the process-wide table-count cap returns 507 with no eviction. Runs with the cap + * set to 1 so the second upload is over the ceiling. + */ +@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) +@DirtiesContext(classMode = DirtiesContext.ClassMode.AFTER_CLASS) +class UploadLimitIntegrationTest { + + private static final ObjectMapper JSON = new ObjectMapper(); + private static final MediaType TEXT_CSV = MediaType.parseMediaType("text/csv"); + + @Autowired + private TestRestTemplate rest; + + private static Path dataDir; + private static Path workDir; + + @DynamicPropertySource + static void properties(DynamicPropertyRegistry registry) throws IOException { + dataDir = Files.createTempDirectory("cuckoo-data"); + Files.createDirectories(dataDir.resolve("data")); + workDir = Files.createTempDirectory("cuckoo-work"); + + registry.add("cuckoodb.data-dir", () -> dataDir.toString()); + registry.add("cuckoodb.work-dir", () -> workDir.toString()); + registry.add("cuckoodb.upload.enabled", () -> "true"); + registry.add("cuckoodb.upload.max-tables", () -> "1"); + } + + @AfterAll + static void cleanup() throws IOException { + deleteRecursively(dataDir); + deleteRecursively(workDir); + } + + @Test + void uploadOverTableCapReturns507() throws Exception { + assertEquals(201, csv("/tables/First", "x\n1\n").getStatusCode().value()); + + ResponseEntity overCap = csv("/tables/Second", "x\n1\n"); + assertEquals(507, overCap.getStatusCode().value(), overCap.getBody()); + assertEquals("TABLE_LIMIT", JSON.readTree(overCap.getBody()).get("errorCode").asText()); + // No eviction: 507 is not retryable, so it carries no Retry-After. + assertFalse(overCap.getHeaders().containsKey("Retry-After")); + } + + private ResponseEntity csv(String path, String body) { + HttpHeaders headers = new HttpHeaders(); + headers.setContentType(TEXT_CSV); + return rest.postForEntity(path, new HttpEntity<>(body, headers), String.class); + } + + private static void deleteRecursively(Path root) throws IOException { + if (root == null || !Files.exists(root)) { + return; + } + try (var paths = Files.walk(root)) { + paths.sorted(Comparator.reverseOrder()).forEach(p -> { + try { + Files.deleteIfExists(p); + } catch (IOException ignored) { + // best-effort temp cleanup + } + }); + } + } +} From 879a64d5200d5f65451b4db85446bca2b439014d Mon Sep 17 00:00:00 2001 From: JinBa1 <72070041+JinBa1@users.noreply.github.com> Date: Fri, 19 Jun 2026 13:41:12 +0100 Subject: [PATCH 2/2] =?UTF-8?q?fix(server):=20address=20PR=20review=20?= =?UTF-8?q?=E2=80=94=20atomic=20table=20cap,=20engine-consistent=20row=20c?= =?UTF-8?q?ount,=20JSON-only=20/queries?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - CatalogFacade.registerIfUnderCap does the count-check + register in one synchronized section, making the upload table-count cap a hard ceiling (was a TOCTOU soft cap) - Extract shared CsvFormats.TABLE dialect (RFC4180 + ignoreSurroundingSpaces) referenced by DBCatalog, ScanOperator, and upload row counting, so rowCount matches SELECT COUNT(*) even for multiline/whitespace-padded quoted fields - GlobalExceptionHandler 415 reports the endpoint's actual accepted media types; pin POST /queries to consumes=application/json, closing silent YAML body acceptance - CatalogFacade.columnsOf returns Optional; dedupe test cleanup into a shared TestFiles helper --- .../github/jinba1/cuckoodb/CsvFormats.java | 22 ++++ .../com/github/jinba1/cuckoodb/DBCatalog.java | 6 +- .../cuckoodb/operator/ScanOperator.java | 7 +- .../server/catalog/CatalogFacade.java | 44 +++++-- .../server/web/GlobalExceptionHandler.java | 14 ++- .../cuckoodb/server/web/QueryController.java | 5 +- .../cuckoodb/server/web/TableController.java | 44 ++++--- .../jinba1/cuckoodb/server/TestFiles.java | 29 +++++ .../server/catalog/CatalogFacadeTest.java | 110 ++++++++++++++++++ .../server/web/BudgetIntegrationTest.java | 17 +-- .../server/web/QueryApiIntegrationTest.java | 17 +-- .../server/web/QueryControllerTest.java | 24 ++++ .../server/web/TableControllerTest.java | 7 +- .../server/web/UploadApiIntegrationTest.java | 38 +++--- .../web/UploadLimitIntegrationTest.java | 17 +-- 15 files changed, 298 insertions(+), 103 deletions(-) create mode 100644 engine/src/main/java/com/github/jinba1/cuckoodb/CsvFormats.java create mode 100644 server/src/test/java/com/github/jinba1/cuckoodb/server/TestFiles.java create mode 100644 server/src/test/java/com/github/jinba1/cuckoodb/server/catalog/CatalogFacadeTest.java diff --git a/engine/src/main/java/com/github/jinba1/cuckoodb/CsvFormats.java b/engine/src/main/java/com/github/jinba1/cuckoodb/CsvFormats.java new file mode 100644 index 0000000..4a4660a --- /dev/null +++ b/engine/src/main/java/com/github/jinba1/cuckoodb/CsvFormats.java @@ -0,0 +1,22 @@ +package com.github.jinba1.cuckoodb; + +import org.apache.commons.csv.CSVFormat; + +/** + * The single CSV dialect every table reader must use, so the catalog's parse-and-infer, the + * {@code ScanOperator}'s row scan, and any external row count all tokenize a file identically. + * RFC4180 with surrounding spaces ignored — keeping these in one constant prevents the readers + * from drifting (e.g. a leading space before a quoted, embedded-newline field changes the record + * count between {@code ignoreSurroundingSpaces} and plain RFC4180). {@link CSVFormat} is immutable + * and thread-safe, so this shared instance is safe to reuse across threads and parsers. + */ +public final class CsvFormats { + + private CsvFormats() { + } + + /** The dialect for every base-table CSV: RFC4180, surrounding spaces ignored. */ + public static final CSVFormat TABLE = CSVFormat.RFC4180.builder() + .setIgnoreSurroundingSpaces(true) + .build(); +} diff --git a/engine/src/main/java/com/github/jinba1/cuckoodb/DBCatalog.java b/engine/src/main/java/com/github/jinba1/cuckoodb/DBCatalog.java index 9ac5549..af23979 100644 --- a/engine/src/main/java/com/github/jinba1/cuckoodb/DBCatalog.java +++ b/engine/src/main/java/com/github/jinba1/cuckoodb/DBCatalog.java @@ -1,6 +1,5 @@ package com.github.jinba1.cuckoodb; -import org.apache.commons.csv.CSVFormat; import org.apache.commons.csv.CSVParser; import org.apache.commons.csv.CSVRecord; @@ -139,10 +138,7 @@ public boolean registerTable(String tableName, Path csv) { * an all-INT table until rows arrive. */ private static TableMeta parseTable(String tableName, Path csv) throws IOException { - CSVFormat format = CSVFormat.RFC4180.builder() - .setIgnoreSurroundingSpaces(true) - .build(); - try (CSVParser parser = CSVParser.parse(csv, StandardCharsets.UTF_8, format)) { + try (CSVParser parser = CSVParser.parse(csv, StandardCharsets.UTF_8, CsvFormats.TABLE)) { Iterator it = parser.iterator(); if (!it.hasNext()) { throw new QueryExecutionException(ErrorCode.DATA_ERROR, diff --git a/engine/src/main/java/com/github/jinba1/cuckoodb/operator/ScanOperator.java b/engine/src/main/java/com/github/jinba1/cuckoodb/operator/ScanOperator.java index 64cee45..b18fdae 100644 --- a/engine/src/main/java/com/github/jinba1/cuckoodb/operator/ScanOperator.java +++ b/engine/src/main/java/com/github/jinba1/cuckoodb/operator/ScanOperator.java @@ -1,6 +1,7 @@ package com.github.jinba1.cuckoodb.operator; import com.github.jinba1.cuckoodb.ColumnType; +import com.github.jinba1.cuckoodb.CsvFormats; import com.github.jinba1.cuckoodb.DBCatalog; import com.github.jinba1.cuckoodb.ErrorCode; import com.github.jinba1.cuckoodb.IntValue; @@ -11,7 +12,6 @@ import com.github.jinba1.cuckoodb.Tuple; import com.github.jinba1.cuckoodb.Value; -import org.apache.commons.csv.CSVFormat; import org.apache.commons.csv.CSVParser; import org.apache.commons.csv.CSVRecord; @@ -65,10 +65,7 @@ public ScanOperator(PlanContext ctx, String tableName) { */ private void openReader() { try { - CSVFormat format = CSVFormat.RFC4180.builder() - .setIgnoreSurroundingSpaces(true) - .build(); - parser = CSVParser.parse(tablePath, StandardCharsets.UTF_8, format); + parser = CSVParser.parse(tablePath, StandardCharsets.UTF_8, CsvFormats.TABLE); records = parser.iterator(); if (records.hasNext()) { records.next(); // skip header row diff --git a/server/src/main/java/com/github/jinba1/cuckoodb/server/catalog/CatalogFacade.java b/server/src/main/java/com/github/jinba1/cuckoodb/server/catalog/CatalogFacade.java index 61dd20a..b514c26 100644 --- a/server/src/main/java/com/github/jinba1/cuckoodb/server/catalog/CatalogFacade.java +++ b/server/src/main/java/com/github/jinba1/cuckoodb/server/catalog/CatalogFacade.java @@ -10,6 +10,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.Optional; /** * The server's only doorway to the engine's static {@link DBCatalog} singleton. Wrapping it @@ -37,13 +38,13 @@ public int tableCount() { /** * The static, catalog-authoritative typed schema of a base table, in column order, or - * {@code null} if the table is absent. Schema discovery must use this — never a query - * result's best-effort inferred types, which are null on an empty result. + * {@link Optional#empty()} if the table is absent. Schema discovery must use this — never a + * query result's best-effort inferred types, which are null on an empty result. */ - public List columnsOf(String name) { + public Optional> columnsOf(String name) { TableMeta meta = DBCatalog.getInstance().getTableMeta(name); if (meta == null) { - return null; + return Optional.empty(); } Map schema = meta.schema(); List types = meta.types(); @@ -58,16 +59,39 @@ public List columnsOf(String name) { for (int i = 0; i < types.size(); i++) { columns.add(new TableColumn(byIndex[i], types.get(i))); } - return columns; + return Optional.of(columns); } /** - * Registers a table from an already-written CSV file. Returns {@code false} when the name - * is already taken (the 409 signal); throws {@code QueryExecutionException(DATA_ERROR)} for - * a malformed CSV. The file must outlive every query that scans the table. + * Atomically enforces the table-count cap and registers a table from an already-written CSV. + * The count check and the register are one critical section, serialized on this singleton bean + * — the sole request-time write path — so concurrent uploads with distinct names cannot each + * pass a separate count check and overshoot the cap. The cap is therefore a hard ceiling, not + * a soft one. + * + *

Returns {@link RegistrationOutcome#OVER_CAP} when the catalog already holds {@code + * maxTables} (nothing is registered), {@link RegistrationOutcome#NAME_TAKEN} when the name is + * already in use (the 409 signal), or {@link RegistrationOutcome#REGISTERED} on success. Throws + * {@code QueryExecutionException(DATA_ERROR)} for a malformed CSV. The file must outlive every + * query that scans the table. */ - public boolean register(String name, Path csv) { - return DBCatalog.getInstance().registerTable(name, csv); + public synchronized RegistrationOutcome registerIfUnderCap(String name, Path csv, int maxTables) { + if (tableCount() >= maxTables) { + return RegistrationOutcome.OVER_CAP; + } + return DBCatalog.getInstance().registerTable(name, csv) + ? RegistrationOutcome.REGISTERED + : RegistrationOutcome.NAME_TAKEN; + } + + /** Result of an {@link #registerIfUnderCap} attempt. */ + public enum RegistrationOutcome { + /** The table was registered under the requested name. */ + REGISTERED, + /** A table of that name already exists; nothing changed (HTTP 409). */ + NAME_TAKEN, + /** The catalog is already at the table-count cap; nothing registered (HTTP 507). */ + OVER_CAP } /** One column of a base table's static schema: bare name + catalog-inferred type. */ diff --git a/server/src/main/java/com/github/jinba1/cuckoodb/server/web/GlobalExceptionHandler.java b/server/src/main/java/com/github/jinba1/cuckoodb/server/web/GlobalExceptionHandler.java index 61f1bcb..1372075 100644 --- a/server/src/main/java/com/github/jinba1/cuckoodb/server/web/GlobalExceptionHandler.java +++ b/server/src/main/java/com/github/jinba1/cuckoodb/server/web/GlobalExceptionHandler.java @@ -12,6 +12,7 @@ import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.http.HttpStatusCode; +import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; import org.springframework.http.converter.HttpMessageNotReadableException; import org.springframework.web.HttpMediaTypeNotSupportedException; @@ -20,7 +21,9 @@ import org.springframework.web.context.request.WebRequest; import org.springframework.web.servlet.mvc.method.annotation.ResponseEntityExceptionHandler; +import java.util.List; import java.util.UUID; +import java.util.stream.Collectors; /** * Translates every failure into the uniform {@code {errorCode, message}} body with the right @@ -128,8 +131,15 @@ public ResponseEntity handleUnexpected(Exception ex) { protected ResponseEntity handleHttpMediaTypeNotSupported( HttpMediaTypeNotSupportedException ex, HttpHeaders headers, HttpStatusCode status, WebRequest request) { - return new ResponseEntity<>(ErrorResponse.of("UNSUPPORTED_MEDIA_TYPE", - "Unsupported Content-Type; this endpoint expects text/csv."), + // This advice is global, so the hint must name THIS endpoint's accepted types (text/csv + // for upload, application/json for /queries) rather than a hardcoded guess. + List supported = ex.getSupportedMediaTypes(); + String message = supported.isEmpty() + ? "Unsupported Content-Type for this endpoint." + : "Unsupported Content-Type; this endpoint expects " + + supported.stream().map(MediaType::toString).collect(Collectors.joining(", ")) + + "."; + return new ResponseEntity<>(ErrorResponse.of("UNSUPPORTED_MEDIA_TYPE", message), HttpStatus.UNSUPPORTED_MEDIA_TYPE); } diff --git a/server/src/main/java/com/github/jinba1/cuckoodb/server/web/QueryController.java b/server/src/main/java/com/github/jinba1/cuckoodb/server/web/QueryController.java index 84abf9b..200a5f3 100644 --- a/server/src/main/java/com/github/jinba1/cuckoodb/server/web/QueryController.java +++ b/server/src/main/java/com/github/jinba1/cuckoodb/server/web/QueryController.java @@ -6,6 +6,7 @@ import com.github.jinba1.cuckoodb.server.web.dto.QueryRequest; import com.github.jinba1.cuckoodb.server.web.dto.QueryResponse; +import org.springframework.http.MediaType; import org.springframework.web.bind.annotation.PostMapping; import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestMapping; @@ -30,7 +31,9 @@ public QueryController(QueryService queryService, ApiPrincipalResolver principal this.principalResolver = principalResolver; } - @PostMapping + // JSON-only gateway: pin consumes so the body is never silently parsed as another on-classpath + // format (e.g. YAML), and so a wrong Content-Type yields a 415 hint naming exactly JSON. + @PostMapping(consumes = MediaType.APPLICATION_JSON_VALUE) public QueryResponse query(@RequestBody QueryRequest request, HttpServletRequest httpRequest) { String principal = principalResolver.resolve(httpRequest); QueryServiceResult result = queryService.execute( diff --git a/server/src/main/java/com/github/jinba1/cuckoodb/server/web/TableController.java b/server/src/main/java/com/github/jinba1/cuckoodb/server/web/TableController.java index b79ee79..eedd1a4 100644 --- a/server/src/main/java/com/github/jinba1/cuckoodb/server/web/TableController.java +++ b/server/src/main/java/com/github/jinba1/cuckoodb/server/web/TableController.java @@ -1,5 +1,6 @@ package com.github.jinba1.cuckoodb.server.web; +import com.github.jinba1.cuckoodb.CsvFormats; import com.github.jinba1.cuckoodb.QueryExecutionException; import com.github.jinba1.cuckoodb.server.catalog.CatalogFacade; import com.github.jinba1.cuckoodb.server.config.CuckooDbProperties; @@ -17,9 +18,12 @@ import jakarta.servlet.http.HttpServletRequest; +import org.apache.commons.csv.CSVParser; + import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardOpenOption; @@ -27,7 +31,6 @@ import java.util.List; import java.util.UUID; import java.util.regex.Pattern; -import java.util.stream.Stream; /** * Table catalog endpoints: list tables, describe a table's static typed schema, and (opt-in) @@ -61,10 +64,8 @@ public List list() { @GetMapping("/{name}") public TableSchemaResponse describe(@PathVariable String name) { validateName(name); - List columns = catalog.columnsOf(name); - if (columns == null) { - throw new TableNotFoundException(name); - } + List columns = catalog.columnsOf(name) + .orElseThrow(() -> new TableNotFoundException(name)); return new TableSchemaResponse(name, toDto(columns)); } @@ -83,8 +84,11 @@ public UploadResponse upload(@PathVariable String name, HttpServletRequest reque throw new UploadDisabledException(); } validateName(name); - if (catalog.tableCount() >= properties.upload().maxTables()) { - throw new TableLimitExceededException(properties.upload().maxTables()); + int maxTables = properties.upload().maxTables(); + // Cheap pre-check to reject an over-cap upload before streaming its body; the authoritative + // check is re-done atomically with the register below, so this is only an optimisation. + if (catalog.tableCount() >= maxTables) { + throw new TableLimitExceededException(maxTables); } Path target = resolveSafeTarget(name); @@ -96,21 +100,28 @@ public UploadResponse upload(@PathVariable String name, HttpServletRequest reque // 500 — that would make the table un-re-uploadable (409) yet reported as failed. long rowCount = countDataRows(target); - boolean registered; + CatalogFacade.RegistrationOutcome outcome; try { - registered = catalog.register(name, target); + outcome = catalog.registerIfUnderCap(name, target, maxTables); } catch (QueryExecutionException e) { // Malformed CSV (DATA_ERROR) on the upload path is a client error (400), not a // 500; strip any server path from the message before returning it. throw new InvalidUploadException(sanitize(e.getMessage(), target)); } - if (!registered) { + // The cap re-check ran under the catalog lock, so a race winner that pushed us to the + // ceiling between the pre-check and here is caught: 507, no overshoot. + if (outcome == CatalogFacade.RegistrationOutcome.OVER_CAP) { + throw new TableLimitExceededException(maxTables); + } + if (outcome == CatalogFacade.RegistrationOutcome.NAME_TAKEN) { throw new TableAlreadyExistsException(name); } // Registration succeeded; nothing below may throw, so the backing file must survive. keep = true; - return new UploadResponse(name, toDto(catalog.columnsOf(name)), rowCount); + List columns = catalog.columnsOf(name).orElseThrow( + () -> new IllegalStateException("Table '" + name + "' vanished after register")); + return new UploadResponse(name, toDto(columns), rowCount); } finally { if (!keep) { Files.deleteIfExists(target); @@ -156,9 +167,16 @@ private static void copyWithCap(InputStream in, Path target, long maxBytes) thro } } + /** + * Data-row count for the upload response. Parses with the engine's exact CSV dialect + * ({@link CsvFormats#TABLE}) rather than counting physical lines, so a quoted field containing + * a newline is one record and the count agrees with what {@code SELECT COUNT(*)} would scan — + * including whitespace-padded multiline quoted fields, which only match when the same + * {@code ignoreSurroundingSpaces} dialect is used. + */ private static long countDataRows(Path csv) throws IOException { - try (Stream lines = Files.lines(csv)) { - return Math.max(0, lines.count() - 1); // drop the header row + try (CSVParser parser = CSVParser.parse(csv, StandardCharsets.UTF_8, CsvFormats.TABLE)) { + return Math.max(0, parser.stream().count() - 1); // first record is the header row } } diff --git a/server/src/test/java/com/github/jinba1/cuckoodb/server/TestFiles.java b/server/src/test/java/com/github/jinba1/cuckoodb/server/TestFiles.java new file mode 100644 index 0000000..ffd669c --- /dev/null +++ b/server/src/test/java/com/github/jinba1/cuckoodb/server/TestFiles.java @@ -0,0 +1,29 @@ +package com.github.jinba1.cuckoodb.server; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Comparator; + +/** Shared test helpers for temp-directory fixtures used by the integration tests. */ +public final class TestFiles { + + private TestFiles() { + } + + /** Best-effort recursive delete of a temp tree; tolerates an already-gone root or files. */ + public static void deleteRecursively(Path root) throws IOException { + if (root == null || !Files.exists(root)) { + return; + } + try (var paths = Files.walk(root)) { + paths.sorted(Comparator.reverseOrder()).forEach(p -> { + try { + Files.deleteIfExists(p); + } catch (IOException ignored) { + // best-effort temp cleanup + } + }); + } + } +} diff --git a/server/src/test/java/com/github/jinba1/cuckoodb/server/catalog/CatalogFacadeTest.java b/server/src/test/java/com/github/jinba1/cuckoodb/server/catalog/CatalogFacadeTest.java new file mode 100644 index 0000000..7f11769 --- /dev/null +++ b/server/src/test/java/com/github/jinba1/cuckoodb/server/catalog/CatalogFacadeTest.java @@ -0,0 +1,110 @@ +package com.github.jinba1.cuckoodb.server.catalog; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import com.github.jinba1.cuckoodb.DBCatalog; +import com.github.jinba1.cuckoodb.server.catalog.CatalogFacade.RegistrationOutcome; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +/** + * Unit tests for the catalog seam, focused on the atomic table-count cap. {@code DBCatalog} is a + * static singleton, so each test resets it; the concurrency test proves the cap is a hard ceiling, + * not a TOCTOU soft cap that concurrent uploads can overshoot. + */ +class CatalogFacadeTest { + + @TempDir + Path tmp; + + private final CatalogFacade catalog = new CatalogFacade(); + + @BeforeEach + void freshEmptyCatalog() { + DBCatalog.resetDBCatalog(); + DBCatalog.getInstance(); // empty, unloaded catalog — no data dir scan + } + + @AfterEach + void clearCatalog() { + DBCatalog.resetDBCatalog(); + } + + private Path csv(String name) throws IOException { + Path p = tmp.resolve(name + ".csv"); + Files.writeString(p, "x\n1\n"); + return p; + } + + @Test + void registersWhenUnderCap() throws IOException { + assertEquals(RegistrationOutcome.REGISTERED, catalog.registerIfUnderCap("A", csv("A"), 5)); + assertTrue(catalog.exists("A")); + assertEquals(1, catalog.tableCount()); + } + + @Test + void reportsNameTakenOnDuplicate() throws IOException { + assertEquals(RegistrationOutcome.REGISTERED, catalog.registerIfUnderCap("Dup", csv("Dup"), 5)); + assertEquals(RegistrationOutcome.NAME_TAKEN, catalog.registerIfUnderCap("Dup", csv("Dup2"), 5)); + assertEquals(1, catalog.tableCount()); + } + + @Test + void refusesAtCapWithoutRegistering() throws IOException { + assertEquals(RegistrationOutcome.REGISTERED, catalog.registerIfUnderCap("One", csv("One"), 1)); + assertEquals(RegistrationOutcome.OVER_CAP, catalog.registerIfUnderCap("Two", csv("Two"), 1)); + assertFalse(catalog.exists("Two")); + assertEquals(1, catalog.tableCount()); + } + + @Test + void concurrentUploadsNeverExceedTheCap() throws Exception { + int cap = 5; + int threads = 32; + ExecutorService pool = Executors.newFixedThreadPool(threads); + CountDownLatch start = new CountDownLatch(1); + AtomicInteger registered = new AtomicInteger(); + List> futures = new ArrayList<>(); + try { + for (int i = 0; i < threads; i++) { + final int id = i; + futures.add(pool.submit(() -> { + start.await(); + Path p = tmp.resolve("T" + id + ".csv"); + Files.writeString(p, "x\n1\n"); + if (catalog.registerIfUnderCap("T" + id, p, cap) == RegistrationOutcome.REGISTERED) { + registered.incrementAndGet(); + } + return null; + })); + } + start.countDown(); + for (Future f : futures) { + f.get(10, TimeUnit.SECONDS); + } + } finally { + pool.shutdownNow(); + } + // Hard ceiling: distinct-named concurrent winners can never push the count past the cap. + assertEquals(cap, catalog.tableCount(), "cap must be a hard ceiling"); + assertEquals(cap, registered.get(), "exactly cap registrations should report REGISTERED"); + } +} diff --git a/server/src/test/java/com/github/jinba1/cuckoodb/server/web/BudgetIntegrationTest.java b/server/src/test/java/com/github/jinba1/cuckoodb/server/web/BudgetIntegrationTest.java index ad24d12..d240f20 100644 --- a/server/src/test/java/com/github/jinba1/cuckoodb/server/web/BudgetIntegrationTest.java +++ b/server/src/test/java/com/github/jinba1/cuckoodb/server/web/BudgetIntegrationTest.java @@ -1,5 +1,6 @@ package com.github.jinba1.cuckoodb.server.web; +import static com.github.jinba1.cuckoodb.server.TestFiles.deleteRecursively; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -8,7 +9,6 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; -import java.util.Comparator; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.Test; @@ -93,19 +93,4 @@ private ResponseEntity postQuery(String json) { headers.setContentType(MediaType.APPLICATION_JSON); return rest.postForEntity("/queries", new HttpEntity<>(json, headers), String.class); } - - private static void deleteRecursively(Path root) throws IOException { - if (root == null || !Files.exists(root)) { - return; - } - try (var paths = Files.walk(root)) { - paths.sorted(Comparator.reverseOrder()).forEach(p -> { - try { - Files.deleteIfExists(p); - } catch (IOException ignored) { - // best-effort temp cleanup - } - }); - } - } } diff --git a/server/src/test/java/com/github/jinba1/cuckoodb/server/web/QueryApiIntegrationTest.java b/server/src/test/java/com/github/jinba1/cuckoodb/server/web/QueryApiIntegrationTest.java index eea7079..3821d06 100644 --- a/server/src/test/java/com/github/jinba1/cuckoodb/server/web/QueryApiIntegrationTest.java +++ b/server/src/test/java/com/github/jinba1/cuckoodb/server/web/QueryApiIntegrationTest.java @@ -1,5 +1,6 @@ package com.github.jinba1.cuckoodb.server.web; +import static com.github.jinba1.cuckoodb.server.TestFiles.deleteRecursively; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -12,7 +13,6 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; -import java.util.Comparator; import java.util.List; import java.util.concurrent.Callable; import java.util.concurrent.ExecutorService; @@ -167,19 +167,4 @@ private ResponseEntity exchange(String path, HttpMethod method, headers.setContentType(contentType); return rest.exchange(path, method, new HttpEntity<>(body, headers), String.class); } - - private static void deleteRecursively(Path root) throws IOException { - if (root == null || !Files.exists(root)) { - return; - } - try (var paths = Files.walk(root)) { - paths.sorted(Comparator.reverseOrder()).forEach(p -> { - try { - Files.deleteIfExists(p); - } catch (IOException ignored) { - // best-effort temp cleanup - } - }); - } - } } diff --git a/server/src/test/java/com/github/jinba1/cuckoodb/server/web/QueryControllerTest.java b/server/src/test/java/com/github/jinba1/cuckoodb/server/web/QueryControllerTest.java index 4afb050..2316f1b 100644 --- a/server/src/test/java/com/github/jinba1/cuckoodb/server/web/QueryControllerTest.java +++ b/server/src/test/java/com/github/jinba1/cuckoodb/server/web/QueryControllerTest.java @@ -213,4 +213,28 @@ void malformedJsonBodyMapsTo400() throws Exception { .andExpect(status().isBadRequest()) .andExpect(jsonPath("$.errorCode").value("BAD_REQUEST")); } + + @Test + void yamlBodyOnQueriesIsRejectedAsJsonOnly() throws Exception { + // /queries is a JSON-only gateway; a YAML body must not be silently parsed even though + // jackson-dataformat-yaml is on the classpath. + mvc.perform(post("/queries").contentType(MediaType.parseMediaType("application/yaml")) + .content("sql: \"SELECT 1\"")) + .andExpect(status().isUnsupportedMediaType()) + .andExpect(jsonPath("$.errorCode").value("UNSUPPORTED_MEDIA_TYPE")) + .andExpect(jsonPath("$.message").value( + org.hamcrest.Matchers.not(org.hamcrest.Matchers.containsString("yaml")))); + } + + @Test + void wrongContentTypeOnQueriesGivesEndpointSpecificHint() throws Exception { + // The 415 handler is global, so its hint must reflect THIS endpoint's accepted types + // (JSON for /queries), never the upload endpoint's text/csv. + mvc.perform(post("/queries").contentType(MediaType.TEXT_PLAIN).content(body("SELECT 1"))) + .andExpect(status().isUnsupportedMediaType()) + .andExpect(jsonPath("$.errorCode").value("UNSUPPORTED_MEDIA_TYPE")) + .andExpect(jsonPath("$.message").value(org.hamcrest.Matchers.containsString("json"))) + .andExpect(jsonPath("$.message").value( + org.hamcrest.Matchers.not(org.hamcrest.Matchers.containsString("text/csv")))); + } } diff --git a/server/src/test/java/com/github/jinba1/cuckoodb/server/web/TableControllerTest.java b/server/src/test/java/com/github/jinba1/cuckoodb/server/web/TableControllerTest.java index a3b27c1..b52dfab 100644 --- a/server/src/test/java/com/github/jinba1/cuckoodb/server/web/TableControllerTest.java +++ b/server/src/test/java/com/github/jinba1/cuckoodb/server/web/TableControllerTest.java @@ -10,6 +10,7 @@ import com.github.jinba1.cuckoodb.server.catalog.CatalogFacade; import java.util.List; +import java.util.Optional; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -46,9 +47,9 @@ void listReturnsSortedNames() throws Exception { @Test void describeReturnsStaticTypedSchema() throws Exception { - when(catalog.columnsOf("Student")).thenReturn(List.of( + when(catalog.columnsOf("Student")).thenReturn(Optional.of(List.of( new CatalogFacade.TableColumn("a", ColumnType.INT), - new CatalogFacade.TableColumn("name", ColumnType.STRING))); + new CatalogFacade.TableColumn("name", ColumnType.STRING)))); mvc.perform(get("/tables/Student")) .andExpect(status().isOk()) .andExpect(jsonPath("$.name").value("Student")) @@ -59,7 +60,7 @@ void describeReturnsStaticTypedSchema() throws Exception { @Test void describeMissingTableReturns404() throws Exception { - when(catalog.columnsOf("Ghost")).thenReturn(null); + when(catalog.columnsOf("Ghost")).thenReturn(Optional.empty()); mvc.perform(get("/tables/Ghost")) .andExpect(status().isNotFound()) .andExpect(jsonPath("$.errorCode").value("UNKNOWN_TABLE")); diff --git a/server/src/test/java/com/github/jinba1/cuckoodb/server/web/UploadApiIntegrationTest.java b/server/src/test/java/com/github/jinba1/cuckoodb/server/web/UploadApiIntegrationTest.java index 70797b5..78f6872 100644 --- a/server/src/test/java/com/github/jinba1/cuckoodb/server/web/UploadApiIntegrationTest.java +++ b/server/src/test/java/com/github/jinba1/cuckoodb/server/web/UploadApiIntegrationTest.java @@ -1,5 +1,6 @@ package com.github.jinba1.cuckoodb.server.web; +import static com.github.jinba1.cuckoodb.server.TestFiles.deleteRecursively; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -10,7 +11,6 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; -import java.util.Comparator; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.Test; @@ -85,6 +85,27 @@ void uploadThenQueryRoundTrips() throws Exception { .get("columns").get(0).get("type").asText(), "sku column is string-typed"); } + @Test + void embeddedNewlineInQuotedFieldCountsAsOneRow() throws Exception { + // A quoted field containing a newline is ONE CSV record per RFC4180, not two lines; + // rowCount must agree with what SELECT COUNT(*) would scan. + ResponseEntity resp = csv("/tables/Notes", "note\n\"line1\nline2\"\n"); + assertEquals(201, resp.getStatusCode().value(), resp.getBody()); + assertEquals(1, JSON.readTree(resp.getBody()).get("rowCount").asInt(), + "embedded-newline quoted field must count as a single data row"); + } + + @Test + void whitespacePaddedMultilineQuotedFieldMatchesEngineRowCount() throws Exception { + // Leading/trailing whitespace around a quoted field with an embedded newline: the engine's + // ignoreSurroundingSpaces dialect treats this as ONE record, so rowCount must too — plain + // RFC4180 would mis-split it into two. + ResponseEntity resp = csv("/tables/Padded", "note\n \"l1\nl2\" \n"); + assertEquals(201, resp.getStatusCode().value(), resp.getBody()); + assertEquals(1, JSON.readTree(resp.getBody()).get("rowCount").asInt(), + "whitespace-padded multiline quoted field must count as one row, matching the engine"); + } + @Test void oversizeUploadReturns413() { String big = "n\n" + "1\n".repeat(40); // ~82 bytes > 64-byte cap @@ -133,19 +154,4 @@ private ResponseEntity exchange(String path, HttpMethod method, headers.setContentType(contentType); return rest.exchange(path, method, new HttpEntity<>(body, headers), String.class); } - - private static void deleteRecursively(Path root) throws IOException { - if (root == null || !Files.exists(root)) { - return; - } - try (var paths = Files.walk(root)) { - paths.sorted(Comparator.reverseOrder()).forEach(p -> { - try { - Files.deleteIfExists(p); - } catch (IOException ignored) { - // best-effort temp cleanup - } - }); - } - } } diff --git a/server/src/test/java/com/github/jinba1/cuckoodb/server/web/UploadLimitIntegrationTest.java b/server/src/test/java/com/github/jinba1/cuckoodb/server/web/UploadLimitIntegrationTest.java index 970a7e5..3b2c52d 100644 --- a/server/src/test/java/com/github/jinba1/cuckoodb/server/web/UploadLimitIntegrationTest.java +++ b/server/src/test/java/com/github/jinba1/cuckoodb/server/web/UploadLimitIntegrationTest.java @@ -1,5 +1,6 @@ package com.github.jinba1.cuckoodb.server.web; +import static com.github.jinba1.cuckoodb.server.TestFiles.deleteRecursively; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -8,7 +9,6 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; -import java.util.Comparator; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.Test; @@ -74,19 +74,4 @@ private ResponseEntity csv(String path, String body) { headers.setContentType(TEXT_CSV); return rest.postForEntity(path, new HttpEntity<>(body, headers), String.class); } - - private static void deleteRecursively(Path root) throws IOException { - if (root == null || !Files.exists(root)) { - return; - } - try (var paths = Files.walk(root)) { - paths.sorted(Comparator.reverseOrder()).forEach(p -> { - try { - Files.deleteIfExists(p); - } catch (IOException ignored) { - // best-effort temp cleanup - } - }); - } - } }