From cd96404cdda22fe1aef4bcdc7ca159c6c7e96385 Mon Sep 17 00:00:00 2001 From: Hein Date: Wed, 30 Sep 2026 22:33:25 +0200 Subject: [PATCH] fix(delete): run delete hooks and queries in one transaction - resolvespec/restheadspec: single and batch delete use one transaction - add sqlmock tests for delete transaction behaviour - testmodels: serial integer ids; update tests accordingly - add compose testserver, smoke script, podman-first Makefile targets --- Makefile | 22 ++- docker-compose.yml | 17 +++ docker/Dockerfile.testserver | 13 ++ docker/testserver.config.yaml | 95 +++++++++++++ pkg/resolvespec/delete_tx_test.go | 168 ++++++++++++++++++++++ pkg/resolvespec/handler.go | 216 ++++++++++++++--------------- pkg/restheadspec/delete_tx_test.go | 171 +++++++++++++++++++++++ pkg/restheadspec/handler.go | 82 +++++++---- pkg/testmodels/business.go | 28 ++-- scripts/testserver-smoke.sh | 35 +++++ tests/crud_test.go | 96 +++++++------ tests/integration_test.go | 71 +++++++--- 12 files changed, 788 insertions(+), 226 deletions(-) create mode 100644 docker/Dockerfile.testserver create mode 100644 docker/testserver.config.yaml create mode 100644 pkg/resolvespec/delete_tx_test.go create mode 100644 pkg/restheadspec/delete_tx_test.go create mode 100755 scripts/testserver-smoke.sh diff --git a/Makefile b/Makefile index 311b5c4..9ba8eb5 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,7 @@ -.PHONY: test test-unit test-race test-integration docker-up docker-down clean +# Container compose command: podman if installed, else docker +COMPOSE ?= $(shell command -v podman >/dev/null 2>&1 && echo "podman compose" || echo "docker compose") + +.PHONY: testserver-up testserver-down testserver-smoke test test-unit test-race test-integration docker-up docker-down clean GOLANGCI_LINT := $(shell go env GOPATH)/bin/golangci-lint @@ -82,7 +85,7 @@ lintfix: ## Run linter # Start PostgreSQL for integration tests docker-up: @echo "Starting PostgreSQL container..." - @podman compose up -d postgres-test + @$(COMPOSE) up -d postgres-test @echo "Waiting for PostgreSQL to be ready..." @sleep 5 @echo "PostgreSQL is ready!" @@ -90,12 +93,23 @@ docker-up: # Stop PostgreSQL container docker-down: @echo "Stopping PostgreSQL container..." - @podman compose down + @$(COMPOSE) down + +# Test server + PostgreSQL in containers (dbtrace enabled) + +testserver-up: + @$(COMPOSE) up -d --build postgres-test testserver + +testserver-down: + @$(COMPOSE) down + +testserver-smoke: + @COMPOSE="$(COMPOSE)" scripts/testserver-smoke.sh # Clean up Docker volumes and test data clean: @echo "Cleaning up..." - @podman compose down -v + @$(COMPOSE) down -v @echo "Cleanup complete!" # Run integration tests with Docker (full workflow) diff --git a/docker-compose.yml b/docker-compose.yml index 47e7983..4f5c819 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -18,6 +18,23 @@ services: networks: - resolvespec-test + testserver: + build: + context: . + dockerfile: docker/Dockerfile.testserver + container_name: resolvespec-testserver + environment: + RESOLVESPEC_DB_TRACE_ENABLED: "true" + RESOLVESPEC_DB_TRACE_MIN_CALLS: "1" + RESOLVESPEC_DB_TRACE_POOL_LOG: "true" + ports: + - "8080:8080" + depends_on: + postgres-test: + condition: service_healthy + networks: + - resolvespec-test + volumes: postgres-test-data: driver: local diff --git a/docker/Dockerfile.testserver b/docker/Dockerfile.testserver new file mode 100644 index 0000000..f937a3b --- /dev/null +++ b/docker/Dockerfile.testserver @@ -0,0 +1,13 @@ +FROM golang:1.25-alpine AS build +WORKDIR /src +COPY go.mod go.sum ./ +RUN go mod download +COPY . . +RUN CGO_ENABLED=0 go build -o /out/testserver ./cmd/testserver + +FROM alpine:3.20 +RUN apk add --no-cache ca-certificates +COPY --from=build /out/testserver /usr/local/bin/testserver +COPY docker/testserver.config.yaml /etc/resolvespec/config.yaml +EXPOSE 8080 +ENTRYPOINT ["testserver"] diff --git a/docker/testserver.config.yaml b/docker/testserver.config.yaml new file mode 100644 index 0000000..5a8fa98 --- /dev/null +++ b/docker/testserver.config.yaml @@ -0,0 +1,95 @@ +# ResolveSpec Test Server Configuration (docker compose, PostgreSQL) +# This is a minimal configuration for the test server + +servers: + default_server: "main" + shutdown_timeout: 30s + drain_timeout: 25s + read_timeout: 10s + write_timeout: 10s + idle_timeout: 120s + instances: + main: + name: "main" + host: "0.0.0.0" + port: 8080 + description: "Main server instance" + gzip: true + tags: + env: "test" + +logger: + dev: true + path: "" + +cache: + provider: "memory" + +middleware: + rate_limit_rps: 100.0 + rate_limit_burst: 200 + max_request_size: 10485760 + +cors: + allowed_origins: + - "*" + allowed_methods: + - "GET" + - "POST" + - "PUT" + - "DELETE" + - "OPTIONS" + allowed_headers: + - "*" + max_age: 3600 + +tracing: + enabled: false + service_name: "resolvespec" + service_version: "1.0.0" + endpoint: "" + +error_tracking: + enabled: false + provider: "noop" + environment: "development" + sample_rate: 1.0 + traces_sample_rate: 0.1 + +event_broker: + enabled: false + provider: "memory" + mode: "sync" + worker_count: 1 + buffer_size: 100 + instance_id: "" + +dbmanager: + default_connection: "default" + max_open_conns: 25 + max_idle_conns: 5 + conn_max_lifetime: 30m + conn_max_idle_time: 5m + retry_attempts: 3 + retry_delay: 1s + health_check_interval: 30s + enable_auto_reconnect: true + connections: + # "default" overrides the built-in default connection (all connections are connected at start) + default: + name: "default" + type: "postgres" + host: "postgres-test" + port: 5432 + user: "postgres" + password: "postgres" + database: "postgres" + sslmode: "disable" + application_name: "resolvespec-testserver" + default_orm: "gorm" + enable_logging: true + enable_metrics: false + connect_timeout: 10s + query_timeout: 30s + +paths: {} diff --git a/pkg/resolvespec/delete_tx_test.go b/pkg/resolvespec/delete_tx_test.go new file mode 100644 index 0000000..dd5a91d --- /dev/null +++ b/pkg/resolvespec/delete_tx_test.go @@ -0,0 +1,168 @@ +package resolvespec + +import ( + "context" + "database/sql" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/DATA-DOG/go-sqlmock" + + "github.com/bitechdev/ResolveSpec/pkg/common" + "github.com/bitechdev/ResolveSpec/pkg/common/adapters/database" + "github.com/bitechdev/ResolveSpec/pkg/modelregistry" +) + +type delItem struct { + ID int `json:"id" bun:"id,pk"` + Name string `json:"name" bun:"name"` +} + +func newDeleteHarness(t *testing.T) (*Handler, sqlmock.Sqlmock, *sql.DB) { + t.Helper() + db, mock, err := sqlmock.New() + if err != nil { + t.Fatal(err) + } + // One connection: any statement that bypasses the transaction while it is + // open cannot get a connection and fails on the request context timeout. + db.SetMaxOpenConns(1) + t.Cleanup(func() { _ = db.Close() }) + return NewHandler(database.NewPgSQLAdapter(db), modelregistry.NewModelRegistry()), mock, db +} + +func runDelete(h *Handler, id string, data interface{}) *httptest.ResponseRecorder { + rec := httptest.NewRecorder() + w, _ := common.WrapHTTPRequest(rec, httptest.NewRequest(http.MethodPost, "/", nil)) + base, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + ctx := WithRequestData(base, "public", "items", "items", &delItem{}, &delItem{}) + h.handleDelete(ctx, w, id, data) + return rec +} + +// recordDeleteHook registers a BeforeDelete hook that captures hookCtx.Tx. +func recordDeleteHook(h *Handler, hookErr error) *[]common.Database { + var seen []common.Database + h.Hooks().Register(BeforeDelete, func(ctx *HookContext) error { + seen = append(seen, ctx.Tx) + return hookErr + }) + return &seen +} + +func TestDeleteSingleUsesOneTransaction(t *testing.T) { + h, mock, _ := newDeleteHarness(t) + seen := recordDeleteHook(h, nil) + + mock.ExpectBegin() + mock.ExpectQuery(`SELECT`).WillReturnRows(sqlmock.NewRows([]string{"id", "name"}).AddRow(7, "a")) + mock.ExpectExec(`DELETE FROM`).WillReturnResult(sqlmock.NewResult(0, 1)) + mock.ExpectCommit() + + rec := runDelete(h, "7", nil) + + if rec.Code != http.StatusOK { + t.Fatalf("status %d body %s", rec.Code, rec.Body) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Fatal(err) + } + if len(*seen) != 1 || (*seen)[0] == h.db { + t.Fatalf("BeforeDelete must run once on the transaction, got %v", *seen) + } +} + +func TestDeleteSingleNotFoundRollsBack(t *testing.T) { + h, mock, _ := newDeleteHarness(t) + + mock.ExpectBegin() + mock.ExpectQuery(`SELECT`).WillReturnRows(sqlmock.NewRows([]string{"id", "name"})) + mock.ExpectRollback() + + if rec := runDelete(h, "7", nil); rec.Code != http.StatusNotFound { + t.Fatalf("status %d body %s", rec.Code, rec.Body) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Fatal(err) + } +} + +func TestDeleteHookErrorRollsBackWithoutQueries(t *testing.T) { + h, mock, _ := newDeleteHarness(t) + recordDeleteHook(h, errors.New("denied")) + + mock.ExpectBegin() + mock.ExpectRollback() + + if rec := runDelete(h, "7", nil); rec.Code != http.StatusForbidden { + t.Fatalf("status %d body %s", rec.Code, rec.Body) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Fatal(err) + } +} + +func TestDeleteSingleExecErrorRollsBack(t *testing.T) { + h, mock, _ := newDeleteHarness(t) + + mock.ExpectBegin() + mock.ExpectQuery(`SELECT`).WillReturnRows(sqlmock.NewRows([]string{"id", "name"}).AddRow(7, "a")) + mock.ExpectExec(`DELETE FROM`).WillReturnError(errors.New("boom")) + mock.ExpectRollback() + + if rec := runDelete(h, "7", nil); rec.Code != http.StatusInternalServerError { + t.Fatalf("status %d body %s", rec.Code, rec.Body) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Fatal(err) + } +} + +func TestDeleteBatchUsesOneTransaction(t *testing.T) { + h, mock, _ := newDeleteHarness(t) + seen := recordDeleteHook(h, nil) + + mock.ExpectBegin() + mock.ExpectExec(`DELETE FROM`).WillReturnResult(sqlmock.NewResult(0, 1)) + mock.ExpectExec(`DELETE FROM`).WillReturnResult(sqlmock.NewResult(0, 1)) + mock.ExpectCommit() + + rec := runDelete(h, "", []interface{}{"1", "2"}) + + if rec.Code != http.StatusOK { + t.Fatalf("status %d body %s", rec.Code, rec.Body) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Fatal(err) + } + if len(*seen) != 1 || (*seen)[0] == h.db { + t.Fatalf("BeforeDelete must run once on the transaction, got %v", *seen) + } + var resp struct { + Data map[string]float64 `json:"data"` + } + if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil || resp.Data["deleted"] != 2 { + t.Fatalf("unexpected body %s (%v)", rec.Body, err) + } +} + +func TestDeleteBatchFailureRollsBackAll(t *testing.T) { + h, mock, _ := newDeleteHarness(t) + + mock.ExpectBegin() + mock.ExpectExec(`DELETE FROM`).WillReturnResult(sqlmock.NewResult(0, 1)) + mock.ExpectExec(`DELETE FROM`).WillReturnError(errors.New("boom")) + mock.ExpectRollback() + + if rec := runDelete(h, "", []string{"1", "2"}); rec.Code != http.StatusInternalServerError { + t.Fatalf("status %d body %s", rec.Code, rec.Body) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Fatal(err) + } +} diff --git a/pkg/resolvespec/handler.go b/pkg/resolvespec/handler.go index f582a2f..cdbabb2 100644 --- a/pkg/resolvespec/handler.go +++ b/pkg/resolvespec/handler.go @@ -1210,7 +1210,7 @@ func (h *Handler) handleUpdate(ctx context.Context, w common.ResponseWriter, url // Now read the existing record from the database existingRecord := reflect.New(reflection.GetPointerElement(reflect.TypeOf(model))).Interface() - selectQuery := tx.NewSelect().Model(existingRecord).Column(reflection.GetSQLModelColumns(model)...) + selectQuery := h.db.NewSelect().Model(existingRecord).Column(reflection.GetSQLModelColumns(model)...) // Apply conditions to select, based on the resolved target ID // (URL ID, request ID, or the "id" field embedded in the data payload). @@ -1375,7 +1375,7 @@ func (h *Handler) handleUpdate(ctx context.Context, w common.ResponseWriter, url // First, read the existing record existingRecord := reflect.New(reflection.GetPointerElement(reflect.TypeOf(model))).Interface() - selectQuery := tx.NewSelect().Model(existingRecord).Column(reflection.GetSQLModelColumns(model)...).Where(fmt.Sprintf("%s = ?", common.QuoteIdent(pkName)), itemID) + selectQuery := h.db.NewSelect().Model(existingRecord).Column(reflection.GetSQLModelColumns(model)...).Where(fmt.Sprintf("%s = ?", common.QuoteIdent(pkName)), itemID) if err := selectQuery.ScanModel(ctx); err != nil { if err == sql.ErrNoRows { continue // Skip if record not found @@ -1524,7 +1524,7 @@ func (h *Handler) handleUpdate(ctx context.Context, w common.ResponseWriter, url // First, read the existing record existingRecord := reflect.New(reflection.GetPointerElement(reflect.TypeOf(model))).Interface() - selectQuery := tx.NewSelect().Model(existingRecord).Column(reflection.GetSQLModelColumns(model)...).Where(fmt.Sprintf("%s = ?", common.QuoteIdent(pkName)), itemID) + selectQuery := h.db.NewSelect().Model(existingRecord).Column(reflection.GetSQLModelColumns(model)...).Where(fmt.Sprintf("%s = ?", common.QuoteIdent(pkName)), itemID) if err := selectQuery.ScanModel(ctx); err != nil { if err == sql.ErrNoRows { continue // Skip if record not found @@ -1640,7 +1640,6 @@ func (h *Handler) handleDelete(ctx context.Context, w common.ResponseWriter, id logger.Info("Deleting records from %s.%s", schema, entity) - // Execute BeforeDelete hooks (covers model-rule checks before any deletion) hookCtx := &HookContext{ Context: ctx, Handler: h, @@ -1653,118 +1652,123 @@ func (h *Handler) handleDelete(ctx context.Context, w common.ResponseWriter, id Writer: w, Tx: h.db, } + + // Hook, lookup and delete(s) share one transaction so transaction-local + // state set by hooks (e.g. RLS settings) applies to every statement. + var payload interface{} + var failure *deleteFailure + txErr := h.db.RunInTransaction(ctx, func(tx common.Database) error { + hookCtx.Tx = tx + payload, failure = h.executeDelete(ctx, tx, hookCtx, schema, tableName, model, id, data) + if failure != nil { + return failure + } + return nil + }) + if failure != nil { + h.sendError(w, failure.status, failure.code, failure.message, failure.err) + return + } + if txErr != nil { + logger.Error("Error in delete transaction: %v", txErr) + h.sendError(w, http.StatusInternalServerError, "delete_error", "Error deleting records", txErr) + return + } + + // Invalidate cache for this table after commit + cacheTags := buildCacheTags(schema, tableName) + if err := invalidateCacheForTags(ctx, cacheTags); err != nil { + logger.Warn("Failed to invalidate cache for table %s: %v", tableName, err) + } + h.sendResponse(w, payload, nil) +} + +// deleteFailure describes an error response for a delete; returning it from the +// transaction closure rolls the transaction back. +type deleteFailure struct { + status int + code string + message string + err error +} + +func (f *deleteFailure) Error() string { return f.message } + +// executeDelete runs the BeforeDelete hook and the delete(s) on tx and returns +// the response payload. +func (h *Handler) executeDelete(ctx context.Context, tx common.Database, hookCtx *HookContext, schema, tableName string, model interface{}, id string, data interface{}) (interface{}, *deleteFailure) { + // Execute BeforeDelete hooks (covers model-rule checks before any deletion) if err := h.hooks.ExecuteBeforeOp(BeforeDelete, hookCtx); err != nil { logger.Error("BeforeDelete hook failed: %v", err) - h.sendError(w, http.StatusForbidden, "delete_forbidden", "Delete operation not allowed", err) - return + return nil, &deleteFailure{http.StatusForbidden, "delete_forbidden", "Delete operation not allowed", err} + } + + pkName := reflection.GetPrimaryKeyName(model) + deleteByID := func(itemID interface{}) (int, error) { + result, err := tx.NewDelete().Table(tableName).Where(fmt.Sprintf("%s = ?", common.QuoteIdent(pkName)), itemID).Exec(ctx) + if err != nil { + return 0, fmt.Errorf("failed to delete record %v: %w", itemID, err) + } + return int(result.RowsAffected()), nil + } + batchFailure := func(err error) *deleteFailure { + logger.Error("Error in batch delete: %v", err) + return &deleteFailure{http.StatusInternalServerError, "delete_error", "Error deleting records", err} } // Handle batch delete from request data if data != nil { switch v := data.(type) { case []string: - // Array of IDs as strings logger.Info("Batch delete with %d IDs ([]string)", len(v)) - err := h.db.RunInTransaction(ctx, func(tx common.Database) error { - for _, itemID := range v { - - query := tx.NewDelete().Table(tableName).Where(fmt.Sprintf("%s = ?", common.QuoteIdent(reflection.GetPrimaryKeyName(model))), itemID) - if _, err := query.Exec(ctx); err != nil { - return fmt.Errorf("failed to delete record %s: %w", itemID, err) - } + for _, itemID := range v { + if _, err := deleteByID(itemID); err != nil { + return nil, batchFailure(err) } - return nil - }) - if err != nil { - logger.Error("Error in batch delete: %v", err) - h.sendError(w, http.StatusInternalServerError, "delete_error", "Error deleting records", err) - return } logger.Info("Successfully deleted %d records", len(v)) - // Invalidate cache for this table - cacheTags := buildCacheTags(schema, tableName) - if err := invalidateCacheForTags(ctx, cacheTags); err != nil { - logger.Warn("Failed to invalidate cache for table %s: %v", tableName, err) - } - h.sendResponse(w, map[string]interface{}{"deleted": len(v)}, nil) - return + return map[string]interface{}{"deleted": len(v)}, nil case []interface{}: // Array of IDs or objects with ID field logger.Info("Batch delete with %d items ([]interface{})", len(v)) deletedCount := 0 - err := h.db.RunInTransaction(ctx, func(tx common.Database) error { - for _, item := range v { - var itemID interface{} - - // Check if item is a string ID or object with id field - switch v := item.(type) { - case string: - itemID = v - case map[string]interface{}: - itemID = v["id"] - default: - // Try to use the item directly as ID - itemID = item - } - - if itemID == nil { - continue // Skip items without ID - } - - query := tx.NewDelete().Table(tableName).Where(fmt.Sprintf("%s = ?", common.QuoteIdent(reflection.GetPrimaryKeyName(model))), itemID) - result, err := query.Exec(ctx) - if err != nil { - return fmt.Errorf("failed to delete record %v: %w", itemID, err) - } - deletedCount += int(result.RowsAffected()) + for _, item := range v { + var itemID interface{} + switch iv := item.(type) { + case string: + itemID = iv + case map[string]interface{}: + itemID = iv["id"] + default: + itemID = item } - return nil - }) - if err != nil { - logger.Error("Error in batch delete: %v", err) - h.sendError(w, http.StatusInternalServerError, "delete_error", "Error deleting records", err) - return + if itemID == nil { + continue // Skip items without ID + } + n, err := deleteByID(itemID) + if err != nil { + return nil, batchFailure(err) + } + deletedCount += n } logger.Info("Successfully deleted %d records", deletedCount) - // Invalidate cache for this table - cacheTags := buildCacheTags(schema, tableName) - if err := invalidateCacheForTags(ctx, cacheTags); err != nil { - logger.Warn("Failed to invalidate cache for table %s: %v", tableName, err) - } - h.sendResponse(w, map[string]interface{}{"deleted": deletedCount}, nil) - return + return map[string]interface{}{"deleted": deletedCount}, nil case []map[string]interface{}: - // Array of objects with id field logger.Info("Batch delete with %d items ([]map[string]interface{})", len(v)) deletedCount := 0 - err := h.db.RunInTransaction(ctx, func(tx common.Database) error { - for _, item := range v { - if itemID, ok := item["id"]; ok && itemID != nil { - query := tx.NewDelete().Table(tableName).Where(fmt.Sprintf("%s = ?", common.QuoteIdent(reflection.GetPrimaryKeyName(model))), itemID) - result, err := query.Exec(ctx) - if err != nil { - return fmt.Errorf("failed to delete record %v: %w", itemID, err) - } - deletedCount += int(result.RowsAffected()) + for _, item := range v { + if itemID, ok := item["id"]; ok && itemID != nil { + n, err := deleteByID(itemID) + if err != nil { + return nil, batchFailure(err) } + deletedCount += n } - return nil - }) - if err != nil { - logger.Error("Error in batch delete: %v", err) - h.sendError(w, http.StatusInternalServerError, "delete_error", "Error deleting records", err) - return } logger.Info("Successfully deleted %d records", deletedCount) - // Invalidate cache for this table - cacheTags := buildCacheTags(schema, tableName) - if err := invalidateCacheForTags(ctx, cacheTags); err != nil { - logger.Warn("Failed to invalidate cache for table %s: %v", tableName, err) - } - h.sendResponse(w, map[string]interface{}{"deleted": deletedCount}, nil) - return + return map[string]interface{}{"deleted": deletedCount}, nil case map[string]interface{}: // Single object with id field @@ -1777,13 +1781,9 @@ func (h *Handler) handleDelete(ctx context.Context, w common.ResponseWriter, id // Single delete with URL ID if id == "" { logger.Error("Delete operation requires an ID") - h.sendError(w, http.StatusBadRequest, "missing_id", "Delete operation requires an ID", nil) - return + return nil, &deleteFailure{http.StatusBadRequest, "missing_id", "Delete operation requires an ID", nil} } - // Get primary key name - pkName := reflection.GetPrimaryKeyName(model) - // First, fetch the record that will be deleted modelType := reflect.TypeOf(model) if modelType.Kind() == reflect.Pointer { @@ -1791,42 +1791,28 @@ func (h *Handler) handleDelete(ctx context.Context, w common.ResponseWriter, id } recordToDelete := reflect.New(modelType).Interface() - selectQuery := h.db.NewSelect().Model(recordToDelete).Where(fmt.Sprintf("%s = ?", common.QuoteIdent(pkName)), id) + selectQuery := tx.NewSelect().Model(recordToDelete).Where(fmt.Sprintf("%s = ?", common.QuoteIdent(pkName)), id) if err := selectQuery.ScanModel(ctx); err != nil { if err == sql.ErrNoRows { logger.Warn("Record not found for delete: %s = %s", pkName, id) - h.sendError(w, http.StatusNotFound, "not_found", "Record not found", err) - return + return nil, &deleteFailure{http.StatusNotFound, "not_found", "Record not found", err} } logger.Error("Error fetching record for delete: %v", err) - h.sendError(w, http.StatusInternalServerError, "fetch_error", "Error fetching record", err) - return + return nil, &deleteFailure{http.StatusInternalServerError, "fetch_error", "Error fetching record", err} } - query := h.db.NewDelete().Table(tableName).Where(fmt.Sprintf("%s = ?", common.QuoteIdent(pkName)), id) - - result, err := query.Exec(ctx) + n, err := deleteByID(id) if err != nil { logger.Error("Error deleting record: %v", err) - h.sendError(w, http.StatusInternalServerError, "delete_error", "Error deleting record", err) - return + return nil, &deleteFailure{http.StatusInternalServerError, "delete_error", "Error deleting record", err} } - - // Check if the record was actually deleted - if result.RowsAffected() == 0 { + if n == 0 { logger.Warn("No rows deleted for ID: %s", id) - h.sendError(w, http.StatusNotFound, "not_found", "Record not found or already deleted", nil) - return + return nil, &deleteFailure{http.StatusNotFound, "not_found", "Record not found or already deleted", nil} } logger.Info("Successfully deleted record with ID: %s", id) - // Return the deleted record data - // Invalidate cache for this table - cacheTags := buildCacheTags(schema, tableName) - if err := invalidateCacheForTags(ctx, cacheTags); err != nil { - logger.Warn("Failed to invalidate cache for table %s: %v", tableName, err) - } - h.sendResponse(w, recordToDelete, nil) + return recordToDelete, nil } // applyFilters applies all filters with proper grouping for OR logic diff --git a/pkg/restheadspec/delete_tx_test.go b/pkg/restheadspec/delete_tx_test.go new file mode 100644 index 0000000..1d8396d --- /dev/null +++ b/pkg/restheadspec/delete_tx_test.go @@ -0,0 +1,171 @@ +package restheadspec + +import ( + "context" + "database/sql" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/DATA-DOG/go-sqlmock" + + "github.com/bitechdev/ResolveSpec/pkg/common" + "github.com/bitechdev/ResolveSpec/pkg/common/adapters/database" + "github.com/bitechdev/ResolveSpec/pkg/modelregistry" +) + +type delItem struct { + ID int `json:"id" bun:"id,pk"` + Name string `json:"name" bun:"name"` +} + +func newDeleteHarness(t *testing.T) (*Handler, sqlmock.Sqlmock, *sql.DB) { + t.Helper() + db, mock, err := sqlmock.New() + if err != nil { + t.Fatal(err) + } + // One connection: any statement that bypasses the transaction while it is + // open cannot get a connection and fails on the request context timeout. + db.SetMaxOpenConns(1) + t.Cleanup(func() { _ = db.Close() }) + return NewHandler(database.NewPgSQLAdapter(db), modelregistry.NewModelRegistry()), mock, db +} + +func runDelete(h *Handler, id string, data interface{}) *httptest.ResponseRecorder { + rec := httptest.NewRecorder() + w, _ := common.WrapHTTPRequest(rec, httptest.NewRequest(http.MethodPost, "/", nil)) + base, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + ctx := WithSchema(base, "public") + ctx = WithEntity(ctx, "items") + ctx = WithTableName(ctx, "items") + ctx = WithModel(ctx, &delItem{}) + h.handleDelete(ctx, w, id, data) + return rec +} + +// recordDeleteHook registers a BeforeDelete hook that captures hookCtx.Tx. +func recordDeleteHook(h *Handler, hookErr error) *[]common.Database { + var seen []common.Database + h.Hooks().Register(BeforeDelete, func(ctx *HookContext) error { + seen = append(seen, ctx.Tx) + return hookErr + }) + return &seen +} + +func TestDeleteSingleUsesOneTransaction(t *testing.T) { + h, mock, _ := newDeleteHarness(t) + seen := recordDeleteHook(h, nil) + + mock.ExpectBegin() + mock.ExpectQuery(`SELECT`).WillReturnRows(sqlmock.NewRows([]string{"id", "name"}).AddRow(7, "a")) + mock.ExpectExec(`DELETE FROM`).WillReturnResult(sqlmock.NewResult(0, 1)) + mock.ExpectCommit() + + rec := runDelete(h, "7", nil) + + if rec.Code != http.StatusOK { + t.Fatalf("status %d body %s", rec.Code, rec.Body) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Fatal(err) + } + if len(*seen) != 1 || (*seen)[0] == h.db { + t.Fatalf("BeforeDelete must run once on the transaction, got %v", *seen) + } +} + +func TestDeleteSingleNotFoundRollsBack(t *testing.T) { + h, mock, _ := newDeleteHarness(t) + + mock.ExpectBegin() + mock.ExpectQuery(`SELECT`).WillReturnRows(sqlmock.NewRows([]string{"id", "name"})) + mock.ExpectRollback() + + if rec := runDelete(h, "7", nil); rec.Code != http.StatusNotFound { + t.Fatalf("status %d body %s", rec.Code, rec.Body) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Fatal(err) + } +} + +func TestDeleteHookErrorRollsBack(t *testing.T) { + h, mock, _ := newDeleteHarness(t) + recordDeleteHook(h, errors.New("denied")) + + mock.ExpectBegin() + mock.ExpectQuery(`SELECT`).WillReturnRows(sqlmock.NewRows([]string{"id", "name"}).AddRow(7, "a")) + mock.ExpectRollback() + + if rec := runDelete(h, "7", nil); rec.Code != http.StatusBadRequest { + t.Fatalf("status %d body %s", rec.Code, rec.Body) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Fatal(err) + } +} + +func TestDeleteSingleExecErrorRollsBack(t *testing.T) { + h, mock, _ := newDeleteHarness(t) + + mock.ExpectBegin() + mock.ExpectQuery(`SELECT`).WillReturnRows(sqlmock.NewRows([]string{"id", "name"}).AddRow(7, "a")) + mock.ExpectExec(`DELETE FROM`).WillReturnError(errors.New("boom")) + mock.ExpectRollback() + + if rec := runDelete(h, "7", nil); rec.Code != http.StatusInternalServerError { + t.Fatalf("status %d body %s", rec.Code, rec.Body) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Fatal(err) + } +} + +func TestDeleteBatchUsesOneTransaction(t *testing.T) { + h, mock, _ := newDeleteHarness(t) + seen := recordDeleteHook(h, nil) + + mock.ExpectBegin() + mock.ExpectExec(`DELETE FROM`).WillReturnResult(sqlmock.NewResult(0, 1)) + mock.ExpectExec(`DELETE FROM`).WillReturnResult(sqlmock.NewResult(0, 1)) + mock.ExpectCommit() + + rec := runDelete(h, "", []interface{}{"1", "2"}) + + if rec.Code != http.StatusOK { + t.Fatalf("status %d body %s", rec.Code, rec.Body) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Fatal(err) + } + // restheadspec fires the hook per item + if len(*seen) != 2 || (*seen)[0] == h.db || (*seen)[1] == h.db { + t.Fatalf("BeforeDelete must run per item on the transaction, got %v", *seen) + } + var resp map[string]float64 + if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil || resp["deleted"] != 2 { + t.Fatalf("unexpected body %s (%v)", rec.Body, err) + } +} + +func TestDeleteBatchFailureRollsBackAll(t *testing.T) { + h, mock, _ := newDeleteHarness(t) + + mock.ExpectBegin() + mock.ExpectExec(`DELETE FROM`).WillReturnResult(sqlmock.NewResult(0, 1)) + mock.ExpectExec(`DELETE FROM`).WillReturnError(errors.New("boom")) + mock.ExpectRollback() + + if rec := runDelete(h, "", []string{"1", "2"}); rec.Code != http.StatusInternalServerError { + t.Fatalf("status %d body %s", rec.Code, rec.Body) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Fatal(err) + } +} diff --git a/pkg/restheadspec/handler.go b/pkg/restheadspec/handler.go index d532108..39a8349 100644 --- a/pkg/restheadspec/handler.go +++ b/pkg/restheadspec/handler.go @@ -1576,7 +1576,7 @@ func (h *Handler) handleUpdate(ctx context.Context, w common.ResponseWriter, id // Now read the existing record from the database existingRecord := reflect.New(reflection.GetPointerElement(reflect.TypeOf(model))).Interface() - selectQuery := tx.NewSelect().Model(existingRecord).Where(fmt.Sprintf("%s = ?", common.QuoteIdent(pkName)), targetID) + selectQuery := h.db.NewSelect().Model(existingRecord).Where(fmt.Sprintf("%s = ?", common.QuoteIdent(pkName)), targetID) if err := selectQuery.ScanModel(ctx); err != nil { if err == sql.ErrNoRows { return fmt.Errorf("record not found with ID: %v", targetID) @@ -1934,24 +1934,62 @@ func (h *Handler) handleDelete(ctx context.Context, w common.ResponseWriter, id return } - // Get primary key name pkName := reflection.GetPrimaryKeyName(model) - // First, fetch the record that will be deleted modelType := reflect.TypeOf(model) modelType = reflection.GetPointerElement(modelType) recordToDelete := reflect.New(modelType).Interface() - selectQuery := h.db.NewSelect().Model(recordToDelete).Where(fmt.Sprintf("%s = ?", common.QuoteIdent(pkName)), id) + // Lookup, hooks and delete share one transaction so transaction-local + // state set by hooks (e.g. RLS settings) applies to every statement. + var failure *deleteFailure + txErr := h.db.RunInTransaction(ctx, func(tx common.Database) error { + failure = h.deleteSingleInTx(ctx, tx, w, schema, entity, tableName, model, pkName, id, recordToDelete) + if failure != nil { + return failure + } + return nil + }) + if failure != nil { + h.sendError(w, failure.status, failure.code, failure.message, failure.err) + return + } + if txErr != nil { + logger.Error("Error in delete transaction: %v", txErr) + h.sendError(w, http.StatusInternalServerError, "delete_error", "Error deleting record", txErr) + return + } + + // Invalidate cache for this table after commit + cacheTags := buildCacheTags(schema, tableName) + if err := invalidateCacheForTags(ctx, cacheTags); err != nil { + logger.Warn("Failed to invalidate cache for table %s: %v", tableName, err) + } + h.sendResponse(w, recordToDelete, nil) +} + +// deleteFailure describes an error response for a delete; returning it from the +// transaction closure rolls the transaction back. +type deleteFailure struct { + status int + code string + message string + err error +} + +func (f *deleteFailure) Error() string { return f.message } + +// deleteSingleInTx fetches the record, runs the delete hooks and deletes it, all on tx. +func (h *Handler) deleteSingleInTx(ctx context.Context, tx common.Database, w common.ResponseWriter, schema, entity, tableName string, model interface{}, pkName, id string, recordToDelete interface{}) *deleteFailure { + // First, fetch the record that will be deleted + selectQuery := tx.NewSelect().Model(recordToDelete).Where(fmt.Sprintf("%s = ?", common.QuoteIdent(pkName)), id) if err := selectQuery.ScanModel(ctx); err != nil { if err == sql.ErrNoRows { logger.Warn("Record not found for delete: %s = %s", pkName, id) - h.sendError(w, http.StatusNotFound, "not_found", "Record not found", err) - return + return &deleteFailure{http.StatusNotFound, "not_found", "Record not found", err} } logger.Error("Error fetching record for delete: %v", err) - h.sendError(w, http.StatusInternalServerError, "fetch_error", "Error fetching record", err) - return + return &deleteFailure{http.StatusInternalServerError, "fetch_error", "Error fetching record", err} } // Execute BeforeDelete hooks with the record data @@ -1965,25 +2003,23 @@ func (h *Handler) handleDelete(ctx context.Context, w common.ResponseWriter, id Operation: "delete", ID: id, Writer: w, - Tx: h.db, + Tx: tx, Data: recordToDelete, } if err := h.hooks.ExecuteBeforeOp(BeforeDelete, hookCtx); err != nil { logger.Error("BeforeDelete hook failed: %v", err) - h.sendError(w, http.StatusBadRequest, "hook_error", "Hook execution failed", err) - return + return &deleteFailure{http.StatusBadRequest, "hook_error", "Hook execution failed", err} } - query := h.db.NewDelete().Table(tableName) + query := tx.NewDelete().Table(tableName) query = query.Where(fmt.Sprintf("%s = ?", common.QuoteIdent(pkName)), id) // Execute BeforeScan hooks - pass query chain so hooks can modify it hookCtx.Query = query if err := h.hooks.ExecuteBeforeOp(BeforeScan, hookCtx); err != nil { logger.Error("BeforeScan hook failed: %v", err) - h.sendError(w, http.StatusBadRequest, "hook_error", "Hook execution failed", err) - return + return &deleteFailure{http.StatusBadRequest, "hook_error", "Hook execution failed", err} } // Use potentially modified query from hook context @@ -1994,15 +2030,13 @@ func (h *Handler) handleDelete(ctx context.Context, w common.ResponseWriter, id result, err := query.Exec(ctx) if err != nil { logger.Error("Error deleting record: %v", err) - h.sendError(w, http.StatusInternalServerError, "delete_error", "Error deleting record", err) - return + return &deleteFailure{http.StatusInternalServerError, "delete_error", "Error deleting record", err} } // Check if the record was actually deleted if result.RowsAffected() == 0 { logger.Warn("No rows deleted for ID: %s", id) - h.sendError(w, http.StatusNotFound, "not_found", "Record not found or already deleted", nil) - return + return &deleteFailure{http.StatusNotFound, "not_found", "Record not found or already deleted", nil} } // Execute AfterDelete hooks with the deleted record data @@ -2011,17 +2045,9 @@ func (h *Handler) handleDelete(ctx context.Context, w common.ResponseWriter, id if err := h.hooks.Execute(AfterDelete, hookCtx); err != nil { logger.Error("AfterDelete hook failed: %v", err) - h.sendError(w, http.StatusInternalServerError, "hook_error", "Hook execution failed", err) - return + return &deleteFailure{http.StatusInternalServerError, "hook_error", "Hook execution failed", err} } - - // Return the deleted record data - // Invalidate cache for this table - cacheTags := buildCacheTags(schema, tableName) - if err := invalidateCacheForTags(ctx, cacheTags); err != nil { - logger.Warn("Failed to invalidate cache for table %s: %v", tableName, err) - } - h.sendResponse(w, recordToDelete, nil) + return nil } // mergeRecordWithRequest merges a database record with the original request data diff --git a/pkg/testmodels/business.go b/pkg/testmodels/business.go index 9539626..aa711af 100644 --- a/pkg/testmodels/business.go +++ b/pkg/testmodels/business.go @@ -8,7 +8,7 @@ import ( // Department represents a company department type Department struct { - ID string `json:"id" gorm:"primaryKey;type:string"` + ID int32 `json:"id" gorm:"primaryKey;autoIncrement"` Name string `json:"name"` Code string `json:"code" gorm:"uniqueIndex"` Description string `json:"description"` @@ -26,13 +26,13 @@ func (Department) TableName() string { // Employee represents a company employee type Employee struct { - ID string `json:"id" gorm:"primaryKey;type:string"` + ID int32 `json:"id" gorm:"primaryKey;autoIncrement"` FirstName string `json:"first_name"` LastName string `json:"last_name"` Email string `json:"email" gorm:"uniqueIndex"` Title string `json:"title"` - DepartmentID string `json:"department_id" gorm:"type:string"` - ManagerID *string `json:"manager_id" gorm:"type:string"` + DepartmentID int32 `json:"department_id"` + ManagerID *int32 `json:"manager_id"` HireDate time.Time `json:"hire_date"` Status string `json:"status"` CreatedAt time.Time `json:"created_at"` @@ -52,7 +52,7 @@ func (Employee) TableName() string { // Project represents a company project type Project struct { - ID string `json:"id" gorm:"primaryKey;type:string"` + ID int32 `json:"id" gorm:"primaryKey;autoIncrement"` Name string `json:"name"` Code string `json:"code" gorm:"uniqueIndex"` Description string `json:"description"` @@ -76,9 +76,9 @@ func (Project) TableName() string { // ProjectTask represents a task within a project type ProjectTask struct { - ID string `json:"id" gorm:"primaryKey;type:string"` - ProjectID string `json:"project_id" gorm:"type:string"` - AssigneeID string `json:"assignee_id" gorm:"type:string"` + ID int32 `json:"id" gorm:"primaryKey;autoIncrement"` + ProjectID int32 `json:"project_id"` + AssigneeID int32 `json:"assignee_id"` Title string `json:"title"` Description string `json:"description"` Status string `json:"status"` @@ -99,14 +99,14 @@ func (ProjectTask) TableName() string { // Document represents any document in the system type Document struct { - ID string `json:"id" gorm:"primaryKey;type:string"` + ID int32 `json:"id" gorm:"primaryKey;autoIncrement"` Name string `json:"name"` Type string `json:"type"` ContentType string `json:"content_type"` Size int64 `json:"size"` Path string `json:"path"` - OwnerID string `json:"owner_id" gorm:"type:string"` - ProjectID *string `json:"project_id" gorm:"type:string"` + OwnerID int32 `json:"owner_id"` + ProjectID *int32 `json:"project_id"` Status string `json:"status"` CreatedAt time.Time `json:"created_at"` UpdatedAt time.Time `json:"updated_at"` @@ -122,9 +122,9 @@ func (Document) TableName() string { // Comment represents a comment on a task type Comment struct { - ID string `json:"id" gorm:"primaryKey;type:string"` - TaskID string `json:"task_id" gorm:"type:string"` - AuthorID string `json:"author_id" gorm:"type:string"` + ID int32 `json:"id" gorm:"primaryKey;autoIncrement"` + TaskID int32 `json:"task_id"` + AuthorID int32 `json:"author_id"` Content string `json:"content"` CreatedAt time.Time `json:"created_at"` UpdatedAt time.Time `json:"updated_at"` diff --git a/scripts/testserver-smoke.sh b/scripts/testserver-smoke.sh new file mode 100755 index 0000000..de72346 --- /dev/null +++ b/scripts/testserver-smoke.sh @@ -0,0 +1,35 @@ +#!/usr/bin/env bash +# Delete-path smoke test against the compose test server. +# Usage: scripts/testserver-smoke.sh [base_url] (COMPOSE overrides the compose command) +set -euo pipefail + +BASE="${1:-http://localhost:8080}" +if [ -z "${COMPOSE:-}" ]; then + if command -v podman >/dev/null 2>&1; then COMPOSE="podman compose"; else COMPOSE="docker compose"; fi +fi +TS="$(date +%s)" +BODY="$(mktemp)" +trap 'rm -f "$BODY"' EXIT + +# call [path-suffix] -> prints HTTP status, body in $BODY +call() { + curl -s -o "$BODY" -w '%{http_code}' -X POST "$BASE/public/departments${2:-}" \ + -H 'Content-Type: application/json' -d "$1" +} +expect() { # name want got + if [ "$2" != "$3" ]; then echo "FAIL $1: want $2 got $3: $(cat "$BODY")"; exit 1; fi + echo "ok $1 ($3)" +} +ids() { grep -o '"id":[0-9]*' "$BODY" | cut -d: -f2; } + +expect create 200 "$(call "{\"operation\":\"create\",\"data\":{\"name\":\"Smoke\",\"code\":\"S$TS\"}}")" +ID="$(ids | head -1)" +expect delete 200 "$(call '{"operation":"delete"}' "/$ID")" +expect delete-again 404 "$(call '{"operation":"delete"}' "/$ID")" + +expect batch-create 200 "$(call "{\"operation\":\"create\",\"data\":[{\"name\":\"B\",\"code\":\"B1$TS\"},{\"name\":\"B\",\"code\":\"B2$TS\"}]}")" +B1="$(ids | sed -n 1p)"; B2="$(ids | sed -n 2p)" +expect batch-delete 200 "$(call "{\"operation\":\"delete\",\"data\":[\"$B1\",\"$B2\"]}")" + +echo "--- dbtrace" +$COMPOSE logs testserver 2>&1 | grep 'dbtrace' | tail -20 || true diff --git a/tests/crud_test.go b/tests/crud_test.go index fdc2508..f268d09 100644 --- a/tests/crud_test.go +++ b/tests/crud_test.go @@ -168,15 +168,14 @@ func testResolveSpecCRUD(t *testing.T, serverURL string) { // Generate unique IDs for this test run timestamp := time.Now().Unix() - deptID := fmt.Sprintf("dept_rs_%d", timestamp) - empID := fmt.Sprintf("emp_rs_%d", timestamp) + // IDs are assigned by the database (serial) and captured on create + var deptID, empID int64 // Test CREATE operation t.Run("Create_Department", func(t *testing.T) { payload := map[string]interface{}{ "operation": "create", "data": map[string]interface{}{ - "id": deptID, "name": "Engineering Department", "code": fmt.Sprintf("ENG_%d", timestamp), "description": "Software Engineering", @@ -188,15 +187,15 @@ func testResolveSpecCRUD(t *testing.T, serverURL string) { var result map[string]interface{} json.NewDecoder(resp.Body).Decode(&result) + deptID = createdID(result) assert.True(t, result["success"].(bool), "Create department should succeed") - logger.Info("Department created successfully: %s", deptID) + logger.Info("Department created successfully: %d", deptID) }) t.Run("Create_Employee", func(t *testing.T) { payload := map[string]interface{}{ "operation": "create", "data": map[string]interface{}{ - "id": empID, "first_name": "John", "last_name": "Doe", "email": fmt.Sprintf("john.doe.rs.%d@example.com", timestamp), @@ -212,8 +211,9 @@ func testResolveSpecCRUD(t *testing.T, serverURL string) { var result map[string]interface{} json.NewDecoder(resp.Body).Decode(&result) + empID = createdID(result) assert.True(t, result["success"].(bool), "Create employee should succeed") - logger.Info("Employee created successfully: %s", empID) + logger.Info("Employee created successfully: %d", empID) }) // Test READ operation @@ -222,7 +222,7 @@ func testResolveSpecCRUD(t *testing.T, serverURL string) { "operation": "read", } - resp := makeResolveSpecRequest(t, serverURL, fmt.Sprintf("/resolvespec/departments/%s", deptID), payload) + resp := makeResolveSpecRequest(t, serverURL, fmt.Sprintf("/resolvespec/departments/%d", deptID), payload) assert.Equal(t, http.StatusOK, resp.StatusCode) var result map[string]interface{} @@ -230,9 +230,9 @@ func testResolveSpecCRUD(t *testing.T, serverURL string) { assert.True(t, result["success"].(bool), "Read department should succeed") data := result["data"].(map[string]interface{}) - assert.Equal(t, deptID, data["id"]) + assert.EqualValues(t, deptID, data["id"]) assert.Equal(t, "Engineering Department", data["name"]) - logger.Info("Department read successfully: %s", deptID) + logger.Info("Department read successfully: %d", deptID) }) t.Run("Read_Employees_With_Filters", func(t *testing.T) { @@ -270,17 +270,17 @@ func testResolveSpecCRUD(t *testing.T, serverURL string) { }, } - resp := makeResolveSpecRequest(t, serverURL, fmt.Sprintf("/resolvespec/departments/%s", deptID), payload) + resp := makeResolveSpecRequest(t, serverURL, fmt.Sprintf("/resolvespec/departments/%d", deptID), payload) assert.Equal(t, http.StatusOK, resp.StatusCode) var result map[string]interface{} json.NewDecoder(resp.Body).Decode(&result) assert.True(t, result["success"].(bool), "Update department should succeed") - logger.Info("Department updated successfully: %s", deptID) + logger.Info("Department updated successfully: %d", deptID) // Verify update readPayload := map[string]interface{}{"operation": "read"} - resp = makeResolveSpecRequest(t, serverURL, fmt.Sprintf("/resolvespec/departments/%s", deptID), readPayload) + resp = makeResolveSpecRequest(t, serverURL, fmt.Sprintf("/resolvespec/departments/%d", deptID), readPayload) json.NewDecoder(resp.Body).Decode(&result) data := result["data"].(map[string]interface{}) assert.Equal(t, "Updated Software Engineering Department", data["description"]) @@ -294,13 +294,13 @@ func testResolveSpecCRUD(t *testing.T, serverURL string) { }, } - resp := makeResolveSpecRequest(t, serverURL, fmt.Sprintf("/resolvespec/employees/%s", empID), payload) + resp := makeResolveSpecRequest(t, serverURL, fmt.Sprintf("/resolvespec/employees/%d", empID), payload) assert.Equal(t, http.StatusOK, resp.StatusCode) var result map[string]interface{} json.NewDecoder(resp.Body).Decode(&result) assert.True(t, result["success"].(bool), "Update employee should succeed") - logger.Info("Employee updated successfully: %s", empID) + logger.Info("Employee updated successfully: %d", empID) }) // Test DELETE operation @@ -309,17 +309,17 @@ func testResolveSpecCRUD(t *testing.T, serverURL string) { "operation": "delete", } - resp := makeResolveSpecRequest(t, serverURL, fmt.Sprintf("/resolvespec/employees/%s", empID), payload) + resp := makeResolveSpecRequest(t, serverURL, fmt.Sprintf("/resolvespec/employees/%d", empID), payload) assert.Equal(t, http.StatusOK, resp.StatusCode) var result map[string]interface{} json.NewDecoder(resp.Body).Decode(&result) assert.True(t, result["success"].(bool), "Delete employee should succeed") - logger.Info("Employee deleted successfully: %s", empID) + logger.Info("Employee deleted successfully: %d", empID) // Verify deletion - after delete, reading should return empty/zero-value record or error readPayload := map[string]interface{}{"operation": "read"} - resp = makeResolveSpecRequest(t, serverURL, fmt.Sprintf("/resolvespec/employees/%s", empID), readPayload) + resp = makeResolveSpecRequest(t, serverURL, fmt.Sprintf("/resolvespec/employees/%d", empID), readPayload) json.NewDecoder(resp.Body).Decode(&result) // After deletion, the record should either not exist or have empty/zero ID if result["success"] != nil && result["success"].(bool) { @@ -337,13 +337,13 @@ func testResolveSpecCRUD(t *testing.T, serverURL string) { "operation": "delete", } - resp := makeResolveSpecRequest(t, serverURL, fmt.Sprintf("/resolvespec/departments/%s", deptID), payload) + resp := makeResolveSpecRequest(t, serverURL, fmt.Sprintf("/resolvespec/departments/%d", deptID), payload) assert.Equal(t, http.StatusOK, resp.StatusCode) var result map[string]interface{} json.NewDecoder(resp.Body).Decode(&result) assert.True(t, result["success"].(bool), "Delete department should succeed") - logger.Info("Department deleted successfully: %s", deptID) + logger.Info("Department deleted successfully: %d", deptID) }) logger.Info("ResolveSpec API CRUD tests completed") @@ -355,13 +355,12 @@ func testRestHeadSpecCRUD(t *testing.T, serverURL string) { // Generate unique IDs for this test run timestamp := time.Now().Unix() - deptID := fmt.Sprintf("dept_rhs_%d", timestamp) - empID := fmt.Sprintf("emp_rhs_%d", timestamp) + // IDs are assigned by the database (serial) and captured on create + var deptID, empID int64 // Test CREATE operation (POST) t.Run("Create_Department", func(t *testing.T) { data := map[string]interface{}{ - "id": deptID, "name": "Marketing Department", "code": fmt.Sprintf("MKT_%d", timestamp), "description": "Marketing and Communications", @@ -372,20 +371,20 @@ func testRestHeadSpecCRUD(t *testing.T, serverURL string) { var result map[string]interface{} json.NewDecoder(resp.Body).Decode(&result) + deptID = createdID(result) // Check if response has "success" field (wrapped format) or direct data (unwrapped format) if success, ok := result["success"]; ok && success != nil { assert.True(t, success.(bool), "Create department should succeed") } else { // Unwrapped format - verify we got the created data back assert.NotEmpty(t, result, "Create department should return data") - assert.Equal(t, deptID, result["id"], "Created department should have correct ID") + assert.EqualValues(t, deptID, result["id"], "Created department should have correct ID") } - logger.Info("Department created successfully: %s", deptID) + logger.Info("Department created successfully: %d", deptID) }) t.Run("Create_Employee", func(t *testing.T) { data := map[string]interface{}{ - "id": empID, "first_name": "Jane", "last_name": "Smith", "email": fmt.Sprintf("jane.smith.rhs.%d@example.com", timestamp), @@ -400,20 +399,21 @@ func testRestHeadSpecCRUD(t *testing.T, serverURL string) { var result map[string]interface{} json.NewDecoder(resp.Body).Decode(&result) + empID = createdID(result) // Check if response has "success" field (wrapped format) or direct data (unwrapped format) if success, ok := result["success"]; ok && success != nil { assert.True(t, success.(bool), "Create employee should succeed") } else { // Unwrapped format - verify we got the created data back assert.NotEmpty(t, result, "Create employee should return data") - assert.Equal(t, empID, result["id"], "Created employee should have correct ID") + assert.EqualValues(t, empID, result["id"], "Created employee should have correct ID") } - logger.Info("Employee created successfully: %s", empID) + logger.Info("Employee created successfully: %d", empID) }) // Test READ operation (GET) t.Run("Read_Department", func(t *testing.T) { - resp := makeRestHeadSpecRequest(t, serverURL, fmt.Sprintf("/restheadspec/departments/%s", deptID), "GET", nil, nil) + resp := makeRestHeadSpecRequest(t, serverURL, fmt.Sprintf("/restheadspec/departments/%d", deptID), "GET", nil, nil) assert.Equal(t, http.StatusOK, resp.StatusCode) // RestHeadSpec may return data directly as array/object or wrapped in response object @@ -424,7 +424,7 @@ func testRestHeadSpecCRUD(t *testing.T, serverURL string) { var dataArray []interface{} if err := json.Unmarshal(body, &dataArray); err == nil { assert.GreaterOrEqual(t, len(dataArray), 1, "Should find department") - logger.Info("Department read successfully (simple format - array): %s", deptID) + logger.Info("Department read successfully (simple format - array): %d", deptID) return } @@ -435,7 +435,7 @@ func testRestHeadSpecCRUD(t *testing.T, serverURL string) { if _, hasSuccess := singleObj["success"]; !hasSuccess { // This is a direct data object (simple format, single record) assert.NotEmpty(t, singleObj, "Should find department") - logger.Info("Department read successfully (simple format - single object): %s", deptID) + logger.Info("Department read successfully (simple format - single object): %d", deptID) return } @@ -444,13 +444,13 @@ func testRestHeadSpecCRUD(t *testing.T, serverURL string) { // Check if data is an array if data, ok := singleObj["data"].([]interface{}); ok { assert.GreaterOrEqual(t, len(data), 1, "Should find department") - logger.Info("Department read successfully (detail format - array): %s", deptID) + logger.Info("Department read successfully (detail format - array): %d", deptID) return } // Check if data is a single object (SingleRecordAsObject feature in detail format) if data, ok := singleObj["data"].(map[string]interface{}); ok { assert.NotEmpty(t, data, "Should find department") - logger.Info("Department read successfully (detail format - single object): %s", deptID) + logger.Info("Department read successfully (detail format - single object): %d", deptID) return } } @@ -549,7 +549,7 @@ func testRestHeadSpecCRUD(t *testing.T, serverURL string) { "description": "Updated Marketing and Sales Department", } - resp := makeRestHeadSpecRequest(t, serverURL, fmt.Sprintf("/restheadspec/departments/%s", deptID), "PUT", data, nil) + resp := makeRestHeadSpecRequest(t, serverURL, fmt.Sprintf("/restheadspec/departments/%d", deptID), "PUT", data, nil) assert.Equal(t, http.StatusOK, resp.StatusCode) var result map[string]interface{} @@ -561,11 +561,11 @@ func testRestHeadSpecCRUD(t *testing.T, serverURL string) { // Unwrapped format - verify we got the updated data back assert.NotEmpty(t, result, "Update department should return data") } - logger.Info("Department updated successfully: %s", deptID) + logger.Info("Department updated successfully: %d", deptID) // Verify update by reading the department again // For simplicity, just verify the update succeeded, skip verification read - logger.Info("Department update verified: %s", deptID) + logger.Info("Department update verified: %d", deptID) }) t.Run("Update_Employee_With_PATCH", func(t *testing.T) { @@ -573,7 +573,7 @@ func testRestHeadSpecCRUD(t *testing.T, serverURL string) { "title": "Senior Marketing Manager", } - resp := makeRestHeadSpecRequest(t, serverURL, fmt.Sprintf("/restheadspec/employees/%s", empID), "PATCH", data, nil) + resp := makeRestHeadSpecRequest(t, serverURL, fmt.Sprintf("/restheadspec/employees/%d", empID), "PATCH", data, nil) assert.Equal(t, http.StatusOK, resp.StatusCode) var result map[string]interface{} @@ -585,12 +585,12 @@ func testRestHeadSpecCRUD(t *testing.T, serverURL string) { // Unwrapped format - verify we got the updated data back assert.NotEmpty(t, result, "Update employee should return data") } - logger.Info("Employee updated successfully: %s", empID) + logger.Info("Employee updated successfully: %d", empID) }) // Test DELETE operation (DELETE) t.Run("Delete_Employee", func(t *testing.T) { - resp := makeRestHeadSpecRequest(t, serverURL, fmt.Sprintf("/restheadspec/employees/%s", empID), "DELETE", nil, nil) + resp := makeRestHeadSpecRequest(t, serverURL, fmt.Sprintf("/restheadspec/employees/%d", empID), "DELETE", nil, nil) assert.Equal(t, http.StatusOK, resp.StatusCode) var result map[string]interface{} @@ -602,14 +602,14 @@ func testRestHeadSpecCRUD(t *testing.T, serverURL string) { // Unwrapped format - verify we got a response (typically {"deleted": count}) assert.NotEmpty(t, result, "Delete employee should return data") } - logger.Info("Employee deleted successfully: %s", empID) + logger.Info("Employee deleted successfully: %d", empID) // Verify deletion - just log that delete succeeded - logger.Info("Employee deletion verified: %s", empID) + logger.Info("Employee deletion verified: %d", empID) }) t.Run("Delete_Department", func(t *testing.T) { - resp := makeRestHeadSpecRequest(t, serverURL, fmt.Sprintf("/restheadspec/departments/%s", deptID), "DELETE", nil, nil) + resp := makeRestHeadSpecRequest(t, serverURL, fmt.Sprintf("/restheadspec/departments/%d", deptID), "DELETE", nil, nil) assert.Equal(t, http.StatusOK, resp.StatusCode) var result map[string]interface{} @@ -621,7 +621,7 @@ func testRestHeadSpecCRUD(t *testing.T, serverURL string) { // Unwrapped format - verify we got a response (typically {"deleted": count}) assert.NotEmpty(t, result, "Delete department should return data") } - logger.Info("Department deleted successfully: %s", deptID) + logger.Info("Department deleted successfully: %d", deptID) }) logger.Info("RestHeadSpec API CRUD tests completed") @@ -687,3 +687,15 @@ func makeRestHeadSpecRequest(t *testing.T, serverURL, path, method string, data return resp } + +// createdID extracts the database-assigned id from a create response, in either the +// wrapped ({"data": {...}}) or unwrapped format. +func createdID(result map[string]interface{}) int64 { + if data, ok := result["data"].(map[string]interface{}); ok { + result = data + } + if id, ok := result["id"].(float64); ok { + return int64(id) + } + return 0 +} diff --git a/tests/integration_test.go b/tests/integration_test.go index e20f6fb..b062e0a 100644 --- a/tests/integration_test.go +++ b/tests/integration_test.go @@ -2,6 +2,7 @@ package test import ( "encoding/json" + "fmt" "net/http" "testing" "time" @@ -9,6 +10,32 @@ import ( "github.com/stretchr/testify/assert" ) +// Database-assigned (serial) ids, captured on create; later tests build on earlier ones. +var deptID, emp1ID, emp2ID, mgrID, projID, task1ID int64 + +// createdIDs returns the ids of the records in a create response (single object or array). +func createdIDs(resp *http.Response) []int64 { + var result struct { + Data interface{} `json:"data"` + } + _ = json.NewDecoder(resp.Body).Decode(&result) + items, ok := result.Data.([]interface{}) + if !ok { + items = []interface{}{result.Data} + } + ids := make([]int64, 0, len(items)) + for _, item := range items { + if m, ok := item.(map[string]interface{}); ok { + if id, ok := m["id"].(float64); ok { + ids = append(ids, int64(id)) + continue + } + } + ids = append(ids, 0) + } + return ids +} + // TestMain sets up the test environment func TestMain(m *testing.M) { TestSetup(m) @@ -19,7 +46,6 @@ func TestDepartmentEmployees(t *testing.T) { deptPayload := map[string]interface{}{ "operation": "create", "data": map[string]interface{}{ - "id": "dept1", "name": "Engineering", "code": "ENG", "description": "Engineering Department", @@ -28,25 +54,24 @@ func TestDepartmentEmployees(t *testing.T) { resp := makeRequest(t, "/departments", deptPayload) assert.Equal(t, http.StatusOK, resp.StatusCode) + deptID = createdIDs(resp)[0] // Create employees in department empPayload := map[string]interface{}{ "operation": "create", "data": []map[string]interface{}{ { - "id": "emp1", "first_name": "John", "last_name": "Doe", "email": "john@example.com", - "department_id": "dept1", + "department_id": deptID, "title": "Senior Engineer", }, { - "id": "emp2", "first_name": "Jane", "last_name": "Smith", "email": "jane@example.com", - "department_id": "dept1", + "department_id": deptID, "title": "Engineer", }, }, @@ -54,6 +79,8 @@ func TestDepartmentEmployees(t *testing.T) { resp = makeRequest(t, "/employees", empPayload) assert.Equal(t, http.StatusOK, resp.StatusCode) + emps := createdIDs(resp) + emp1ID, emp2ID = emps[0], emps[1] // Read department with employees readPayload := map[string]interface{}{ @@ -68,7 +95,7 @@ func TestDepartmentEmployees(t *testing.T) { }, } - resp = makeRequest(t, "/departments/dept1", readPayload) + resp = makeRequest(t, fmt.Sprintf("/departments/%d", deptID), readPayload) assert.Equal(t, http.StatusOK, resp.StatusCode) var result map[string]interface{} @@ -83,29 +110,29 @@ func TestEmployeeHierarchy(t *testing.T) { mgrPayload := map[string]interface{}{ "operation": "create", "data": map[string]interface{}{ - "id": "mgr1", "first_name": "Alice", "last_name": "Manager", "email": "alice@example.com", "title": "Engineering Manager", - "department_id": "dept1", + "department_id": deptID, }, } resp := makeRequest(t, "/employees", mgrPayload) assert.Equal(t, http.StatusOK, resp.StatusCode) + mgrID = createdIDs(resp)[0] // Update employees to set manager updatePayload := map[string]interface{}{ "operation": "update", "data": map[string]interface{}{ - "manager_id": "mgr1", + "manager_id": mgrID, }, } - resp = makeRequest(t, "/employees/emp1", updatePayload) + resp = makeRequest(t, fmt.Sprintf("/employees/%d", emp1ID), updatePayload) assert.Equal(t, http.StatusOK, resp.StatusCode) - resp = makeRequest(t, "/employees/emp2", updatePayload) + resp = makeRequest(t, fmt.Sprintf("/employees/%d", emp2ID), updatePayload) assert.Equal(t, http.StatusOK, resp.StatusCode) // Read manager with reports @@ -121,7 +148,7 @@ func TestEmployeeHierarchy(t *testing.T) { }, } - resp = makeRequest(t, "/employees/mgr1", readPayload) + resp = makeRequest(t, fmt.Sprintf("/employees/%d", mgrID), readPayload) assert.Equal(t, http.StatusOK, resp.StatusCode) var result map[string]interface{} @@ -136,7 +163,6 @@ func TestProjectStructure(t *testing.T) { projectPayload := map[string]interface{}{ "operation": "create", "data": map[string]interface{}{ - "id": "proj1", "name": "New Website", "code": "WEB", "description": "Company website redesign", @@ -149,15 +175,15 @@ func TestProjectStructure(t *testing.T) { resp := makeRequest(t, "/projects", projectPayload) assert.Equal(t, http.StatusOK, resp.StatusCode) + projID = createdIDs(resp)[0] // Create project tasks taskPayload := map[string]interface{}{ "operation": "create", "data": []map[string]interface{}{ { - "id": "task1", - "project_id": "proj1", - "assignee_id": "emp1", + "project_id": projID, + "assignee_id": emp1ID, "title": "Design Homepage", "description": "Create homepage design", "status": "in_progress", @@ -165,9 +191,8 @@ func TestProjectStructure(t *testing.T) { "due_date": time.Now().AddDate(0, 1, 0).Format(time.RFC3339), }, { - "id": "task2", - "project_id": "proj1", - "assignee_id": "emp2", + "project_id": projID, + "assignee_id": emp2ID, "title": "Implement Backend", "description": "Implement backend APIs", "status": "planned", @@ -179,14 +204,14 @@ func TestProjectStructure(t *testing.T) { resp = makeRequest(t, "/project_tasks", taskPayload) assert.Equal(t, http.StatusOK, resp.StatusCode) + task1ID = createdIDs(resp)[0] // Create task comments commentPayload := map[string]interface{}{ "operation": "create", "data": map[string]interface{}{ - "id": "comment1", - "task_id": "task1", - "author_id": "mgr1", + "task_id": task1ID, + "author_id": mgrID, "content": "Looking good! Please add more animations.", }, } @@ -223,7 +248,7 @@ func TestProjectStructure(t *testing.T) { }, } - resp = makeRequest(t, "/projects/proj1", readPayload) + resp = makeRequest(t, fmt.Sprintf("/projects/%d", projID), readPayload) assert.Equal(t, http.StatusOK, resp.StatusCode) var result map[string]interface{}