From 6af7d35f14002ad83f612716c3398ca76241759f Mon Sep 17 00:00:00 2001 From: Tiago Farto Date: Wed, 20 May 2026 12:38:57 +0000 Subject: [PATCH 1/5] chore: tenant data bulk delete --- cmd/api/app.go | 9 +- cmd/api/app_test.go | 1 + .../api/handlers/feedback_records_handler.go | 14 +- .../handlers/feedback_records_handler_test.go | 69 +++-- internal/api/handlers/tenant_data_handler.go | 63 ++++ .../api/handlers/tenant_data_handler_test.go | 113 +++++++ internal/models/feedback_records.go | 12 +- internal/models/tenant_data.go | 24 ++ .../repository/feedback_records_repository.go | 10 +- .../feedback_records_repository_test.go | 8 +- internal/repository/tenant_data_repository.go | 111 +++++++ .../repository/tenant_data_repository_test.go | 254 ++++++++++++++++ internal/repository/webhooks_repository.go | 4 + internal/service/feedback_records_service.go | 35 ++- .../service/feedback_records_service_test.go | 115 +++++-- internal/service/id_validation.go | 48 +++ internal/service/tenant_data_service.go | 48 +++ internal/service/tenant_data_service_test.go | 146 +++++++++ internal/service/tenant_validation.go | 24 -- openapi.yaml | 106 ++++++- tests/README.md | 2 +- tests/integration_test.go | 282 +++++++++++++++--- 22 files changed, 1343 insertions(+), 155 deletions(-) create mode 100644 internal/api/handlers/tenant_data_handler.go create mode 100644 internal/api/handlers/tenant_data_handler_test.go create mode 100644 internal/models/tenant_data.go create mode 100644 internal/repository/tenant_data_repository.go create mode 100644 internal/repository/tenant_data_repository_test.go create mode 100644 internal/service/id_validation.go create mode 100644 internal/service/tenant_data_service.go create mode 100644 internal/service/tenant_data_service_test.go delete mode 100644 internal/service/tenant_validation.go diff --git a/cmd/api/app.go b/cmd/api/app.go index 581180f..529ebd6 100644 --- a/cmd/api/app.go +++ b/cmd/api/app.go @@ -229,6 +229,7 @@ func NewApp(cfg *config.Config, db *pgxpool.Pool) (*App, error) { feedbackRecordsRepo := repository.NewFeedbackRecordsRepository(db) embeddingsRepo := repository.NewEmbeddingsRepository(db) + tenantDataRepo := repository.NewTenantDataRepository(db) embeddingProviderName, embeddingModel := embeddingProviderAndModel(cfg) embeddingModelForDB := embeddingModel @@ -317,6 +318,8 @@ func NewApp(cfg *config.Config, db *pgxpool.Pool) (*App, error) { webhooksService := service.NewWebhooksService(webhooksRepo, messageManager, cfg.Webhook.MaxCount, cfg.Webhook.URLBlacklist) webhooksHandler := handlers.NewWebhooksHandler(webhooksService) + tenantDataService := service.NewTenantDataService(tenantDataRepo) + tenantDataHandler := handlers.NewTenantDataHandler(tenantDataService) feedbackRecordsHandler := handlers.NewFeedbackRecordsHandler(feedbackRecordsService) healthHandler := handlers.NewHealthHandler() @@ -329,7 +332,7 @@ func NewApp(cfg *config.Config, db *pgxpool.Pool) (*App, error) { } server := newHTTPServer( - cfg, healthHandler, openapiHandler, feedbackRecordsHandler, webhooksHandler, searchHandler, + cfg, healthHandler, openapiHandler, feedbackRecordsHandler, webhooksHandler, tenantDataHandler, searchHandler, meterProvider, tracerProvider, ) @@ -353,6 +356,7 @@ func newHTTPServer( openapi *handlers.OpenAPIHandler, feedback *handlers.FeedbackRecordsHandler, webhooks *handlers.WebhooksHandler, + tenantData *handlers.TenantDataHandler, search *handlers.SearchHandler, meterProvider *sdkmetric.MeterProvider, tracerProvider *sdktrace.TracerProvider, @@ -368,13 +372,14 @@ func newHTTPServer( protected.HandleFunc("GET /v1/feedback-records/{id}", feedback.Get) protected.HandleFunc("PATCH /v1/feedback-records/{id}", feedback.Update) protected.HandleFunc("DELETE /v1/feedback-records/{id}", feedback.Delete) - protected.HandleFunc("DELETE /v1/feedback-records", feedback.BulkDelete) + protected.HandleFunc("DELETE /v1/feedback-records", feedback.DeleteByUser) protected.HandleFunc("POST /v1/webhooks", webhooks.Create) protected.HandleFunc("GET /v1/webhooks", webhooks.List) protected.HandleFunc("GET /v1/webhooks/{id}", webhooks.Get) protected.HandleFunc("PATCH /v1/webhooks/{id}", webhooks.Update) protected.HandleFunc("DELETE /v1/webhooks/{id}", webhooks.Delete) + protected.HandleFunc("DELETE /v1/tenants/{tenant_id}/data", tenantData.Delete) // Search endpoints are always registered; when embeddings are disabled, the handler returns 503. protected.HandleFunc("POST /v1/feedback-records/search/semantic", search.SemanticSearch) diff --git a/cmd/api/app_test.go b/cmd/api/app_test.go index bf5d603..7a2c117 100644 --- a/cmd/api/app_test.go +++ b/cmd/api/app_test.go @@ -262,6 +262,7 @@ func newTestHTTPServerWithPublicBaseURL(t *testing.T, publicBaseURL string) *htt newTestOpenAPIHandler(t, publicBaseURL), handlers.NewFeedbackRecordsHandler(nil), handlers.NewWebhooksHandler(nil), + handlers.NewTenantDataHandler(nil), handlers.NewSearchHandler(nil), nil, nil, diff --git a/internal/api/handlers/feedback_records_handler.go b/internal/api/handlers/feedback_records_handler.go index 4c1363f..d016fe3 100644 --- a/internal/api/handlers/feedback_records_handler.go +++ b/internal/api/handlers/feedback_records_handler.go @@ -25,7 +25,7 @@ type FeedbackRecordsService interface { ListFeedbackRecords(ctx context.Context, filters *models.ListFeedbackRecordsFilters) (*models.ListFeedbackRecordsResponse, error) UpdateFeedbackRecord(ctx context.Context, id uuid.UUID, req *models.UpdateFeedbackRecordRequest) (*models.FeedbackRecord, error) DeleteFeedbackRecord(ctx context.Context, id uuid.UUID) error - BulkDeleteFeedbackRecords(ctx context.Context, filters *models.BulkDeleteFilters) (int, error) + DeleteFeedbackRecordsByUser(ctx context.Context, filters *models.DeleteFeedbackRecordsByUserFilters) (int, error) } // FeedbackRecordsHandler handles HTTP requests for feedback records. @@ -226,9 +226,9 @@ func (h *FeedbackRecordsHandler) Delete(w http.ResponseWriter, r *http.Request) w.WriteHeader(http.StatusNoContent) } -// BulkDelete handles DELETE /v1/feedback-records?user_id=[&tenant_id=]. -func (h *FeedbackRecordsHandler) BulkDelete(w http.ResponseWriter, r *http.Request) { - filters := &models.BulkDeleteFilters{} +// DeleteByUser handles DELETE /v1/feedback-records?user_id=[&tenant_id=]. +func (h *FeedbackRecordsHandler) DeleteByUser(w http.ResponseWriter, r *http.Request) { + filters := &models.DeleteFeedbackRecordsByUserFilters{} // Decode and validate query parameters if err := validation.ValidateAndDecodeQueryParams(r, filters); err != nil { @@ -237,7 +237,7 @@ func (h *FeedbackRecordsHandler) BulkDelete(w http.ResponseWriter, r *http.Reque return } - deletedCount, err := h.service.BulkDeleteFeedbackRecords(r.Context(), filters) + deletedCount, err := h.service.DeleteFeedbackRecordsByUser(r.Context(), filters) if err != nil { if errors.Is(err, huberrors.ErrValidation) { validation.RespondValidationError(w, err) @@ -250,7 +250,7 @@ func (h *FeedbackRecordsHandler) BulkDelete(w http.ResponseWriter, r *http.Reque tenantID = *filters.TenantID } - slog.Error("Failed to bulk delete feedback records", // #nosec G706 -- slog key-values + slog.Error("Failed to delete feedback records by user", // #nosec G706 -- slog key-values "method", r.Method, "path", r.URL.Path, "user_id", filters.UserID, @@ -263,7 +263,7 @@ func (h *FeedbackRecordsHandler) BulkDelete(w http.ResponseWriter, r *http.Reque return } - resp := models.BulkDeleteResponse{ + resp := models.DeleteFeedbackRecordsByUserResponse{ DeletedCount: int64(deletedCount), Message: fmt.Sprintf("Successfully deleted %d feedback records", deletedCount), } diff --git a/internal/api/handlers/feedback_records_handler_test.go b/internal/api/handlers/feedback_records_handler_test.go index 91c3b6c..184a241 100644 --- a/internal/api/handlers/feedback_records_handler_test.go +++ b/internal/api/handlers/feedback_records_handler_test.go @@ -6,6 +6,7 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "strings" "testing" "github.com/google/uuid" @@ -19,8 +20,8 @@ import ( // mockFeedbackRecordsService mocks FeedbackRecordsService for handler tests. type mockFeedbackRecordsService struct { - createFunc func(ctx context.Context, req *models.CreateFeedbackRecordRequest) (*models.FeedbackRecord, error) - bulkDeleteFunc func(ctx context.Context, filters *models.BulkDeleteFilters) (int, error) + createFunc func(ctx context.Context, req *models.CreateFeedbackRecordRequest) (*models.FeedbackRecord, error) + deleteByUserFunc func(ctx context.Context, filters *models.DeleteFeedbackRecordsByUserFilters) (int, error) } func (m *mockFeedbackRecordsService) CreateFeedbackRecord( @@ -53,11 +54,11 @@ func (m *mockFeedbackRecordsService) DeleteFeedbackRecord(context.Context, uuid. return nil } -func (m *mockFeedbackRecordsService) BulkDeleteFeedbackRecords( - ctx context.Context, filters *models.BulkDeleteFilters, +func (m *mockFeedbackRecordsService) DeleteFeedbackRecordsByUser( + ctx context.Context, filters *models.DeleteFeedbackRecordsByUserFilters, ) (int, error) { - if m.bulkDeleteFunc != nil { - return m.bulkDeleteFunc(ctx, filters) + if m.deleteByUserFunc != nil { + return m.deleteByUserFunc(ctx, filters) } return 0, nil @@ -200,10 +201,10 @@ func feedbackRecordCreateBody(t *testing.T, tenantID string) *bytes.Reader { return bytes.NewReader(body) } -func TestFeedbackRecordsHandler_BulkDelete(t *testing.T) { +func TestFeedbackRecordsHandler_DeleteByUser(t *testing.T) { t.Run("success returns 200 with deleted_count and message", func(t *testing.T) { mock := &mockFeedbackRecordsService{ - bulkDeleteFunc: func(_ context.Context, filters *models.BulkDeleteFilters) (int, error) { + deleteByUserFunc: func(_ context.Context, filters *models.DeleteFeedbackRecordsByUserFilters) (int, error) { assert.Equal(t, "user-123", filters.UserID) assert.Nil(t, filters.TenantID) @@ -216,11 +217,11 @@ func TestFeedbackRecordsHandler_BulkDelete(t *testing.T) { http.MethodDelete, "http://test/v1/feedback-records?user_id=user-123", http.NoBody) rec := httptest.NewRecorder() - handler.BulkDelete(rec, req) + handler.DeleteByUser(rec, req) assert.Equal(t, http.StatusOK, rec.Code) - var resp models.BulkDeleteResponse + var resp models.DeleteFeedbackRecordsByUserResponse err := json.Unmarshal(rec.Body.Bytes(), &resp) require.NoError(t, err) @@ -230,7 +231,7 @@ func TestFeedbackRecordsHandler_BulkDelete(t *testing.T) { t.Run("optional tenant_id query parameter is passed to service", func(t *testing.T) { mock := &mockFeedbackRecordsService{ - bulkDeleteFunc: func(_ context.Context, filters *models.BulkDeleteFilters) (int, error) { + deleteByUserFunc: func(_ context.Context, filters *models.DeleteFeedbackRecordsByUserFilters) (int, error) { assert.Equal(t, "user-456", filters.UserID) require.NotNil(t, filters.TenantID) assert.Equal(t, "tenant-a", *filters.TenantID) @@ -244,7 +245,7 @@ func TestFeedbackRecordsHandler_BulkDelete(t *testing.T) { http.MethodDelete, "http://test/v1/feedback-records?user_id=user-456&tenant_id=tenant-a", http.NoBody) rec := httptest.NewRecorder() - handler.BulkDelete(rec, req) + handler.DeleteByUser(rec, req) assert.Equal(t, http.StatusOK, rec.Code) }) @@ -257,7 +258,21 @@ func TestFeedbackRecordsHandler_BulkDelete(t *testing.T) { http.MethodDelete, "http://test/v1/feedback-records?user_id=user-123&tenant_id=", http.NoBody) rec := httptest.NewRecorder() - handler.BulkDelete(rec, req) + handler.DeleteByUser(rec, req) + + assert.Equal(t, http.StatusBadRequest, rec.Code) + }) + + t.Run("overlength tenant_id returns bad request", func(t *testing.T) { + mock := &mockFeedbackRecordsService{} + handler := NewFeedbackRecordsHandler(mock) + tenantID := strings.Repeat("a", 256) + + req := httptest.NewRequestWithContext(context.Background(), + http.MethodDelete, "http://test/v1/feedback-records?user_id=user-123&tenant_id="+tenantID, http.NoBody) + rec := httptest.NewRecorder() + + handler.DeleteByUser(rec, req) assert.Equal(t, http.StatusBadRequest, rec.Code) }) @@ -269,7 +284,7 @@ func TestFeedbackRecordsHandler_BulkDelete(t *testing.T) { req := httptest.NewRequestWithContext(context.Background(), http.MethodDelete, "http://test/v1/feedback-records", http.NoBody) rec := httptest.NewRecorder() - handler.BulkDelete(rec, req) + handler.DeleteByUser(rec, req) assert.Equal(t, http.StatusBadRequest, rec.Code) }) @@ -282,14 +297,28 @@ func TestFeedbackRecordsHandler_BulkDelete(t *testing.T) { http.MethodDelete, "http://test/v1/feedback-records?user_id=", http.NoBody) rec := httptest.NewRecorder() - handler.BulkDelete(rec, req) + handler.DeleteByUser(rec, req) + + assert.Equal(t, http.StatusBadRequest, rec.Code) + }) + + t.Run("overlength user_id returns bad request", func(t *testing.T) { + mock := &mockFeedbackRecordsService{} + handler := NewFeedbackRecordsHandler(mock) + userID := strings.Repeat("a", 256) + + req := httptest.NewRequestWithContext(context.Background(), + http.MethodDelete, "http://test/v1/feedback-records?user_id="+userID, http.NoBody) + rec := httptest.NewRecorder() + + handler.DeleteByUser(rec, req) assert.Equal(t, http.StatusBadRequest, rec.Code) }) t.Run("service error returns 500", func(t *testing.T) { mock := &mockFeedbackRecordsService{ - bulkDeleteFunc: func(_ context.Context, _ *models.BulkDeleteFilters) (int, error) { + deleteByUserFunc: func(_ context.Context, _ *models.DeleteFeedbackRecordsByUserFilters) (int, error) { return 0, assert.AnError }, } @@ -299,14 +328,14 @@ func TestFeedbackRecordsHandler_BulkDelete(t *testing.T) { http.MethodDelete, "http://test/v1/feedback-records?user_id=user-789", http.NoBody) rec := httptest.NewRecorder() - handler.BulkDelete(rec, req) + handler.DeleteByUser(rec, req) assert.Equal(t, http.StatusInternalServerError, rec.Code) }) t.Run("zero deleted returns 200 with deleted_count 0", func(t *testing.T) { mock := &mockFeedbackRecordsService{ - bulkDeleteFunc: func(_ context.Context, _ *models.BulkDeleteFilters) (int, error) { + deleteByUserFunc: func(_ context.Context, _ *models.DeleteFeedbackRecordsByUserFilters) (int, error) { return 0, nil }, } @@ -316,11 +345,11 @@ func TestFeedbackRecordsHandler_BulkDelete(t *testing.T) { http.MethodDelete, "http://test/v1/feedback-records?user_id=nonexistent", http.NoBody) rec := httptest.NewRecorder() - handler.BulkDelete(rec, req) + handler.DeleteByUser(rec, req) assert.Equal(t, http.StatusOK, rec.Code) - var resp models.BulkDeleteResponse + var resp models.DeleteFeedbackRecordsByUserResponse err := json.Unmarshal(rec.Body.Bytes(), &resp) require.NoError(t, err) diff --git a/internal/api/handlers/tenant_data_handler.go b/internal/api/handlers/tenant_data_handler.go new file mode 100644 index 0000000..5e5cdbc --- /dev/null +++ b/internal/api/handlers/tenant_data_handler.go @@ -0,0 +1,63 @@ +package handlers + +import ( + "context" + "errors" + "log/slog" + "net/http" + + "github.com/formbricks/hub/internal/api/response" + "github.com/formbricks/hub/internal/api/validation" + "github.com/formbricks/hub/internal/huberrors" + "github.com/formbricks/hub/internal/models" +) + +// TenantDataService defines the interface for tenant data purge business logic. +type TenantDataService interface { + DeleteTenantData(ctx context.Context, tenantID string) (*models.TenantDataDeleteResult, error) +} + +// TenantDataHandler handles tenant data purge requests. +type TenantDataHandler struct { + service TenantDataService +} + +// NewTenantDataHandler creates a new tenant data handler. +func NewTenantDataHandler(service TenantDataService) *TenantDataHandler { + return &TenantDataHandler{service: service} +} + +// Delete handles DELETE /v1/tenants/{tenant_id}/data. +func (h *TenantDataHandler) Delete(w http.ResponseWriter, r *http.Request) { + tenantID := r.PathValue("tenant_id") + + result, err := h.service.DeleteTenantData(r.Context(), tenantID) + if err != nil { + if errors.Is(err, huberrors.ErrValidation) { + validation.RespondValidationError(w, err) + + return + } + + slog.Error("Failed to delete tenant data", // #nosec G706 -- slog key-values + "method", r.Method, + "path", r.URL.Path, + "tenant_id", tenantID, + "error", err, + ) + + response.RespondInternalServerError(w, "An unexpected error occurred") + + return + } + + resp := models.TenantDataDeleteResponse{ + TenantID: result.TenantID, + DeletedFeedbackRecords: result.DeletedFeedbackRecords, + DeletedEmbeddings: result.DeletedEmbeddings, + DeletedWebhooks: result.DeletedWebhooks, + Message: "Successfully deleted tenant data for " + result.TenantID, + } + + response.RespondJSON(w, http.StatusOK, resp) +} diff --git a/internal/api/handlers/tenant_data_handler_test.go b/internal/api/handlers/tenant_data_handler_test.go new file mode 100644 index 0000000..5d2371b --- /dev/null +++ b/internal/api/handlers/tenant_data_handler_test.go @@ -0,0 +1,113 @@ +package handlers + +import ( + "context" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/formbricks/hub/internal/api/response" + "github.com/formbricks/hub/internal/huberrors" + "github.com/formbricks/hub/internal/models" +) + +type mockTenantDataService struct { + deleteFunc func(ctx context.Context, tenantID string) (*models.TenantDataDeleteResult, error) +} + +func (m *mockTenantDataService) DeleteTenantData( + ctx context.Context, tenantID string, +) (*models.TenantDataDeleteResult, error) { + if m.deleteFunc != nil { + return m.deleteFunc(ctx, tenantID) + } + + return nil, nil +} + +func TestTenantDataHandler_Delete(t *testing.T) { + t.Run("success returns counts", func(t *testing.T) { + mock := &mockTenantDataService{ + deleteFunc: func(_ context.Context, tenantID string) (*models.TenantDataDeleteResult, error) { + assert.Equal(t, "org-123", tenantID) + + return &models.TenantDataDeleteResult{ + TenantID: "org-123", + TenantDataDeleteCounts: models.TenantDataDeleteCounts{ + DeletedFeedbackRecords: 3, + DeletedEmbeddings: 2, + DeletedWebhooks: 1, + }, + }, nil + }, + } + handler := NewTenantDataHandler(mock) + req := httptest.NewRequestWithContext(context.Background(), http.MethodDelete, "http://test/v1/tenants/org-123/data", http.NoBody) + req.SetPathValue("tenant_id", "org-123") + + rec := httptest.NewRecorder() + + handler.Delete(rec, req) + + assert.Equal(t, http.StatusOK, rec.Code) + + var resp models.TenantDataDeleteResponse + + err := json.Unmarshal(rec.Body.Bytes(), &resp) + require.NoError(t, err) + assert.Equal(t, "org-123", resp.TenantID) + assert.Equal(t, int64(3), resp.DeletedFeedbackRecords) + assert.Equal(t, int64(2), resp.DeletedEmbeddings) + assert.Equal(t, int64(1), resp.DeletedWebhooks) + assert.Equal(t, "Successfully deleted tenant data for org-123", resp.Message) + }) + + t.Run("validation error returns bad request", func(t *testing.T) { + mock := &mockTenantDataService{ + deleteFunc: func(context.Context, string) (*models.TenantDataDeleteResult, error) { + return nil, huberrors.NewValidationError("tenant_id", "tenant_id is required and cannot be empty") + }, + } + handler := NewTenantDataHandler(mock) + req := httptest.NewRequestWithContext(context.Background(), http.MethodDelete, "http://test/v1/tenants//data", http.NoBody) + req.SetPathValue("tenant_id", "") + + rec := httptest.NewRecorder() + + handler.Delete(rec, req) + + assert.Equal(t, http.StatusBadRequest, rec.Code) + assert.Contains(t, rec.Header().Get("Content-Type"), "application/problem+json") + + var problem response.ProblemDetails + + err := json.Unmarshal(rec.Body.Bytes(), &problem) + require.NoError(t, err) + assert.Equal(t, response.ProblemTypeValidationError, problem.Type) + require.Len(t, problem.Errors, 1) + assert.Equal(t, "tenant_id", problem.Errors[0].Location) + }) + + t.Run("unexpected service error returns internal server error", func(t *testing.T) { + mock := &mockTenantDataService{ + deleteFunc: func(context.Context, string) (*models.TenantDataDeleteResult, error) { + return nil, errors.New("database unavailable") + }, + } + handler := NewTenantDataHandler(mock) + req := httptest.NewRequestWithContext(context.Background(), http.MethodDelete, "http://test/v1/tenants/org-123/data", http.NoBody) + req.SetPathValue("tenant_id", "org-123") + + rec := httptest.NewRecorder() + + handler.Delete(rec, req) + + assert.Equal(t, http.StatusInternalServerError, rec.Code) + assert.Contains(t, rec.Header().Get("Content-Type"), "application/problem+json") + }) +} diff --git a/internal/models/feedback_records.go b/internal/models/feedback_records.go index 1fe88b9..b44eefd 100644 --- a/internal/models/feedback_records.go +++ b/internal/models/feedback_records.go @@ -246,14 +246,14 @@ type ListFeedbackRecordsResponse struct { NextCursor string `json:"next_cursor,omitempty"` // present when there may be more results } -// BulkDeleteFilters represents query parameters for bulk delete operation. -type BulkDeleteFilters struct { - UserID string `form:"user_id" validate:"required,no_null_bytes,min=1"` - TenantID *string `form:"tenant_id" validate:"omitempty,no_null_bytes,min=1"` +// DeleteFeedbackRecordsByUserFilters represents query parameters for deleting feedback records by user. +type DeleteFeedbackRecordsByUserFilters struct { + UserID string `form:"user_id" validate:"required,no_null_bytes,min=1,max=255"` + TenantID *string `form:"tenant_id" validate:"omitempty,no_null_bytes,min=1,max=255"` } -// BulkDeleteResponse represents the response for bulk delete operation. -type BulkDeleteResponse struct { +// DeleteFeedbackRecordsByUserResponse represents the response for deleting feedback records by user. +type DeleteFeedbackRecordsByUserResponse struct { DeletedCount int64 `json:"deleted_count"` Message string `json:"message"` } diff --git a/internal/models/tenant_data.go b/internal/models/tenant_data.go new file mode 100644 index 0000000..f2cac00 --- /dev/null +++ b/internal/models/tenant_data.go @@ -0,0 +1,24 @@ +package models + +// TenantDataDeleteCounts is the repository result for a tenant data purge. +type TenantDataDeleteCounts struct { + DeletedFeedbackRecords int64 + DeletedEmbeddings int64 + DeletedWebhooks int64 +} + +// TenantDataDeleteResult is the service result for a tenant data purge. +type TenantDataDeleteResult struct { + TenantDataDeleteCounts + + TenantID string +} + +// TenantDataDeleteResponse represents the response for deleting all Hub-owned tenant data. +type TenantDataDeleteResponse struct { + TenantID string `json:"tenant_id"` + DeletedFeedbackRecords int64 `json:"deleted_feedback_records"` + DeletedEmbeddings int64 `json:"deleted_embeddings"` + DeletedWebhooks int64 `json:"deleted_webhooks"` + Message string `json:"message"` +} diff --git a/internal/repository/feedback_records_repository.go b/internal/repository/feedback_records_repository.go index 5739813..4f1cf54 100644 --- a/internal/repository/feedback_records_repository.go +++ b/internal/repository/feedback_records_repository.go @@ -394,11 +394,11 @@ func (r *FeedbackRecordsRepository) Delete(ctx context.Context, id uuid.UUID) er return nil } -// BulkDelete deletes all feedback records matching user_id. +// DeleteByUser deletes all feedback records matching user_id. // When tenant_id is provided, deletion is restricted to that tenant; otherwise all user records are deleted. // It returns deleted IDs grouped by tenant so callers can publish tenant-scoped side effects. -func (r *FeedbackRecordsRepository) BulkDelete( - ctx context.Context, filters *models.BulkDeleteFilters, +func (r *FeedbackRecordsRepository) DeleteByUser( + ctx context.Context, filters *models.DeleteFeedbackRecordsByUserFilters, ) ([]models.DeletedFeedbackRecordsByTenant, error) { query := ` DELETE FROM feedback_records @@ -416,7 +416,7 @@ func (r *FeedbackRecordsRepository) BulkDelete( rows, err := r.db.Query(ctx, query, args...) if err != nil { - return nil, fmt.Errorf("failed to bulk delete feedback records: %w", err) + return nil, fmt.Errorf("failed to delete feedback records by user: %w", err) } defer rows.Close() @@ -444,7 +444,7 @@ func (r *FeedbackRecordsRepository) BulkDelete( } if err := rows.Err(); err != nil { - return nil, fmt.Errorf("error iterating bulk delete result: %w", err) + return nil, fmt.Errorf("error iterating delete feedback records by user result: %w", err) } return groups, nil diff --git a/internal/repository/feedback_records_repository_test.go b/internal/repository/feedback_records_repository_test.go index d54efe0..cfac6b0 100644 --- a/internal/repository/feedback_records_repository_test.go +++ b/internal/repository/feedback_records_repository_test.go @@ -4,11 +4,11 @@ import ( "testing" ) -// BulkDelete is tested by integration tests in tests/integration_test.go: -// - TestFeedbackRecordsRepository_BulkDelete exercises the repository directly and asserts +// DeleteByUser is tested by integration tests in tests/integration_test.go: +// - TestFeedbackRecordsRepository_DeleteByUser exercises the repository directly and asserts // the optional tenant filter and tenant-grouped return values. -// - TestBulkDeleteFeedbackRecords exercises the full stack (handler, service, repo) including +// - TestDeleteFeedbackRecordsByUser exercises the full stack (handler, service, repo) including // tenant-scoped deletion, GDPR user_id erasure across tenants, and response shape. func TestFeedbackRecordsRepository_Package(_ *testing.T) { - // No DB in unit tests; BulkDelete coverage is in tests/. + // No DB in unit tests; DeleteByUser coverage is in tests/. } diff --git a/internal/repository/tenant_data_repository.go b/internal/repository/tenant_data_repository.go new file mode 100644 index 0000000..2945177 --- /dev/null +++ b/internal/repository/tenant_data_repository.go @@ -0,0 +1,111 @@ +package repository + +import ( + "context" + "errors" + "fmt" + "log/slog" + + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgconn" + "github.com/jackc/pgx/v5/pgxpool" + + "github.com/formbricks/hub/internal/models" +) + +type tenantDataExecutor interface { + Exec(ctx context.Context, sql string, arguments ...any) (pgconn.CommandTag, error) +} + +type tenantDataTx interface { + tenantDataExecutor + Commit(ctx context.Context) error + Rollback(ctx context.Context) error +} + +type tenantDataTxBeginner interface { + BeginTx(ctx context.Context, txOptions pgx.TxOptions) (tenantDataTx, error) +} + +type tenantDataPool struct { + db *pgxpool.Pool +} + +func (p tenantDataPool) BeginTx(ctx context.Context, txOptions pgx.TxOptions) (tenantDataTx, error) { + dbTx, err := p.db.BeginTx(ctx, txOptions) + if err != nil { + return nil, fmt.Errorf("begin tenant data transaction: %w", err) + } + + return dbTx, nil +} + +// TenantDataRepository handles tenant-scoped data purge operations. +type TenantDataRepository struct { + db tenantDataTxBeginner +} + +// NewTenantDataRepository creates a new tenant data repository. +func NewTenantDataRepository(db *pgxpool.Pool) *TenantDataRepository { + return &TenantDataRepository{db: tenantDataPool{db: db}} +} + +// DeleteByTenant deletes all Hub-owned data for a tenant and returns per-resource counts. +func (r *TenantDataRepository) DeleteByTenant(ctx context.Context, tenantID string) (*models.TenantDataDeleteCounts, error) { + dbTx, err := r.db.BeginTx(ctx, pgx.TxOptions{}) + if err != nil { + return nil, fmt.Errorf("begin tenant data delete transaction: %w", err) + } + + defer func() { + if err := dbTx.Rollback(ctx); err != nil && !errors.Is(err, pgx.ErrTxClosed) { + slog.Error("tenant data delete: rollback failed", "tenant_id", tenantID, "error", err) + } + }() + + counts, err := deleteTenantDataInTx(ctx, dbTx, tenantID) + if err != nil { + return nil, err + } + + if err := dbTx.Commit(ctx); err != nil { + slog.Error("tenant data delete: commit failed", "tenant_id", tenantID, "error", err) + + return nil, fmt.Errorf("commit tenant data delete transaction: %w", err) + } + + return counts, nil +} + +func deleteTenantDataInTx( + ctx context.Context, exec tenantDataExecutor, tenantID string, +) (*models.TenantDataDeleteCounts, error) { + embeddingTag, err := exec.Exec(ctx, ` + DELETE FROM embeddings e + USING feedback_records fr + WHERE e.feedback_record_id = fr.id + AND fr.tenant_id = $1`, tenantID) + if err != nil { + return nil, fmt.Errorf("delete tenant embeddings: %w", err) + } + + feedbackRecordsTag, err := exec.Exec(ctx, ` + DELETE FROM feedback_records + WHERE tenant_id = $1`, tenantID) + if err != nil { + return nil, fmt.Errorf("delete tenant feedback records: %w", err) + } + + webhooksTag, err := exec.Exec(ctx, ` + DELETE FROM webhooks + WHERE tenant_id = $1`, tenantID) + if err != nil { + return nil, fmt.Errorf("delete tenant webhooks: %w", err) + } + + return &models.TenantDataDeleteCounts{ + DeletedFeedbackRecords: feedbackRecordsTag.RowsAffected(), + DeletedEmbeddings: embeddingTag.RowsAffected(), + DeletedWebhooks: webhooksTag.RowsAffected(), + }, nil +} diff --git a/internal/repository/tenant_data_repository_test.go b/internal/repository/tenant_data_repository_test.go new file mode 100644 index 0000000..8e38436 --- /dev/null +++ b/internal/repository/tenant_data_repository_test.go @@ -0,0 +1,254 @@ +package repository + +import ( + "context" + "errors" + "strings" + "testing" + + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgconn" +) + +type fakeTenantDataDB struct { + tx *fakeTenantDataTx + beginErr error +} + +func (f *fakeTenantDataDB) BeginTx(context.Context, pgx.TxOptions) (tenantDataTx, error) { + if f.beginErr != nil { + return nil, f.beginErr + } + + return f.tx, nil +} + +type fakeTenantDataExecutor struct { + tags []pgconn.CommandTag + errAtQuery int + err error + queries []string + args [][]any +} + +func (f *fakeTenantDataExecutor) Exec( + _ context.Context, sql string, arguments ...any, +) (pgconn.CommandTag, error) { + f.queries = append(f.queries, sql) + f.args = append(f.args, arguments) + + if f.errAtQuery == len(f.queries) { + if f.err != nil { + return pgconn.CommandTag{}, f.err + } + + return pgconn.CommandTag{}, errors.New("exec failed") + } + + tagIndex := len(f.queries) - 1 + if tagIndex >= len(f.tags) { + return pgconn.CommandTag{}, nil + } + + return f.tags[tagIndex], nil +} + +type fakeTenantDataTx struct { + fakeTenantDataExecutor + + commitErr error + rollbackErr error + committed bool + rolledBack bool +} + +func (f *fakeTenantDataTx) Commit(context.Context) error { + f.committed = true + + return f.commitErr +} + +func (f *fakeTenantDataTx) Rollback(context.Context) error { + f.rolledBack = true + + return f.rollbackErr +} + +func TestTenantDataRepository_DeleteByTenant(t *testing.T) { + t.Run("commits transaction and returns counts", func(t *testing.T) { + transaction := &fakeTenantDataTx{ + fakeTenantDataExecutor: fakeTenantDataExecutor{ + tags: []pgconn.CommandTag{ + pgconn.NewCommandTag("DELETE 2"), + pgconn.NewCommandTag("DELETE 3"), + pgconn.NewCommandTag("DELETE 1"), + }, + }, + rollbackErr: pgx.ErrTxClosed, + } + repo := &TenantDataRepository{db: &fakeTenantDataDB{tx: transaction}} + + counts, err := repo.DeleteByTenant(context.Background(), "org-123") + if err != nil { + t.Fatalf("DeleteByTenant() error = %v", err) + } + + if counts.DeletedEmbeddings != 2 || counts.DeletedFeedbackRecords != 3 || counts.DeletedWebhooks != 1 { + t.Fatalf("counts = %+v, want embeddings=2 feedback=3 webhooks=1", counts) + } + + if !transaction.committed { + t.Fatal("transaction was not committed") + } + + if !transaction.rolledBack { + t.Fatal("deferred rollback was not called") + } + }) + + t.Run("rolls back and returns delete error", func(t *testing.T) { + transaction := &fakeTenantDataTx{ + fakeTenantDataExecutor: fakeTenantDataExecutor{errAtQuery: 2}, + } + repo := &TenantDataRepository{db: &fakeTenantDataDB{tx: transaction}} + + counts, err := repo.DeleteByTenant(context.Background(), "org-123") + if err == nil { + t.Fatal("DeleteByTenant() error = nil, want error") + } + + if counts != nil { + t.Fatalf("counts = %+v, want nil", counts) + } + + if transaction.committed { + t.Fatal("transaction was committed after delete error") + } + + if !transaction.rolledBack { + t.Fatal("transaction was not rolled back") + } + }) + + t.Run("returns begin transaction error", func(t *testing.T) { + beginErr := errors.New("begin failed") + repo := &TenantDataRepository{db: &fakeTenantDataDB{beginErr: beginErr}} + + counts, err := repo.DeleteByTenant(context.Background(), "org-123") + if !errors.Is(err, beginErr) { + t.Fatalf("DeleteByTenant() error = %v, want begin error", err) + } + + if counts != nil { + t.Fatalf("counts = %+v, want nil", counts) + } + }) + + t.Run("returns commit error", func(t *testing.T) { + commitErr := errors.New("commit failed") + transaction := &fakeTenantDataTx{ + fakeTenantDataExecutor: fakeTenantDataExecutor{ + tags: []pgconn.CommandTag{ + pgconn.NewCommandTag("DELETE 2"), + pgconn.NewCommandTag("DELETE 3"), + pgconn.NewCommandTag("DELETE 1"), + }, + }, + commitErr: commitErr, + rollbackErr: pgx.ErrTxClosed, + } + repo := &TenantDataRepository{db: &fakeTenantDataDB{tx: transaction}} + + counts, err := repo.DeleteByTenant(context.Background(), "org-123") + if !errors.Is(err, commitErr) { + t.Fatalf("DeleteByTenant() error = %v, want commit error", err) + } + + if counts != nil { + t.Fatalf("counts = %+v, want nil", counts) + } + }) +} + +func TestDeleteTenantDataInTx(t *testing.T) { + t.Run("deletes explicit tenant owned tables and returns counts", func(t *testing.T) { + exec := &fakeTenantDataExecutor{ + tags: []pgconn.CommandTag{ + pgconn.NewCommandTag("DELETE 2"), + pgconn.NewCommandTag("DELETE 3"), + pgconn.NewCommandTag("DELETE 1"), + }, + } + + counts, err := deleteTenantDataInTx(context.Background(), exec, "org-123") + if err != nil { + t.Fatalf("deleteTenantDataInTx() error = %v", err) + } + + if counts.DeletedEmbeddings != 2 || counts.DeletedFeedbackRecords != 3 || counts.DeletedWebhooks != 1 { + t.Fatalf("counts = %+v, want embeddings=2 feedback=3 webhooks=1", counts) + } + + if len(exec.queries) != 3 { + t.Fatalf("queries = %d, want 3", len(exec.queries)) + } + + assertQueryContains(t, exec.queries[0], "DELETE FROM embeddings") + assertQueryContains(t, exec.queries[0], "USING feedback_records") + assertQueryContains(t, exec.queries[1], "DELETE FROM feedback_records") + assertQueryContains(t, exec.queries[2], "DELETE FROM webhooks") + + for queryIndex, args := range exec.args { + if len(args) != 1 || args[0] != "org-123" { + t.Fatalf("query %d args = %#v, want tenant id", queryIndex, args) + } + } + }) + + t.Run("stops after embeddings delete error", func(t *testing.T) { + exec := &fakeTenantDataExecutor{errAtQuery: 1} + + counts, err := deleteTenantDataInTx(context.Background(), exec, "org-123") + if err == nil { + t.Fatal("deleteTenantDataInTx() error = nil, want error") + } + + if counts != nil { + t.Fatalf("counts = %+v, want nil", counts) + } + + if len(exec.queries) != 1 { + t.Fatalf("queries = %d, want 1", len(exec.queries)) + } + }) + + t.Run("stops before webhooks after feedback delete error", func(t *testing.T) { + exec := &fakeTenantDataExecutor{ + tags: []pgconn.CommandTag{ + pgconn.NewCommandTag("DELETE 2"), + }, + errAtQuery: 2, + } + + counts, err := deleteTenantDataInTx(context.Background(), exec, "org-123") + if err == nil { + t.Fatal("deleteTenantDataInTx() error = nil, want error") + } + + if counts != nil { + t.Fatalf("counts = %+v, want nil", counts) + } + + if len(exec.queries) != 2 { + t.Fatalf("queries = %d, want 2", len(exec.queries)) + } + }) +} + +func assertQueryContains(t *testing.T, query, want string) { + t.Helper() + + if !strings.Contains(query, want) { + t.Fatalf("query %q does not contain %q", query, want) + } +} diff --git a/internal/repository/webhooks_repository.go b/internal/repository/webhooks_repository.go index 2a32a13..e6baaad 100644 --- a/internal/repository/webhooks_repository.go +++ b/internal/repository/webhooks_repository.go @@ -28,6 +28,10 @@ func NewWebhooksRepository(db *pgxpool.Pool) *WebhooksRepository { // Create inserts a new webhook. func (r *WebhooksRepository) Create(ctx context.Context, req *models.CreateWebhookRequest) (*models.Webhook, error) { + if req.TenantID == nil { + return nil, huberrors.NewValidationError("tenant_id", "tenant_id is required") + } + enabled := true if req.Enabled != nil { enabled = *req.Enabled diff --git a/internal/service/feedback_records_service.go b/internal/service/feedback_records_service.go index 61b792e..0a3a34c 100644 --- a/internal/service/feedback_records_service.go +++ b/internal/service/feedback_records_service.go @@ -13,12 +13,13 @@ import ( "github.com/riverqueue/river" "github.com/formbricks/hub/internal/datatypes" + "github.com/formbricks/hub/internal/huberrors" "github.com/formbricks/hub/internal/models" "github.com/formbricks/hub/pkg/cursor" ) -// ErrUserIDRequired is returned when bulk delete is called without user_id (err113). -var ErrUserIDRequired = errors.New("user_id is required") +// ErrUserIDRequired is returned when deleting feedback records by user is called without user_id. +var ErrUserIDRequired = huberrors.NewValidationError("user_id", "user_id is required") // ErrEmbeddingBackfillNotConfigured is returned when BackfillEmbeddings is called without embedding inserter/queue. var ErrEmbeddingBackfillNotConfigured = errors.New("embedding backfill not configured") @@ -36,7 +37,7 @@ type FeedbackRecordsRepository interface { ) ([]models.FeedbackRecord, bool, error) Update(ctx context.Context, id uuid.UUID, req *models.UpdateFeedbackRecordRequest) (*models.FeedbackRecord, error) Delete(ctx context.Context, id uuid.UUID) error - BulkDelete(ctx context.Context, filters *models.BulkDeleteFilters) ([]models.DeletedFeedbackRecordsByTenant, error) + DeleteByUser(ctx context.Context, filters *models.DeleteFeedbackRecordsByUserFilters) ([]models.DeletedFeedbackRecordsByTenant, error) } // EmbeddingsRepository defines the interface for embeddings table access. @@ -212,29 +213,37 @@ func (s *FeedbackRecordsService) DeleteFeedbackRecord(ctx context.Context, id uu return nil } -// BulkDeleteFeedbackRecords deletes all feedback records matching user_id. +// DeleteFeedbackRecordsByUser deletes all feedback records matching user_id. // When tenant_id is provided, deletion is restricted to that tenant; otherwise all user records are deleted. // It publishes one tenant-aware FeedbackRecordDeleted event per tenant represented in the deleted rows. -func (s *FeedbackRecordsService) BulkDeleteFeedbackRecords(ctx context.Context, filters *models.BulkDeleteFilters) (int, error) { - if filters == nil || filters.UserID == "" { +func (s *FeedbackRecordsService) DeleteFeedbackRecordsByUser( + ctx context.Context, filters *models.DeleteFeedbackRecordsByUserFilters, +) (int, error) { + if filters == nil { return 0, ErrUserIDRequired } + normalizedUserID, err := normalizeRequiredUserIDValue(filters.UserID) + if err != nil { + return 0, err + } + + normalizedFilters := &models.DeleteFeedbackRecordsByUserFilters{ + UserID: normalizedUserID, + } + if filters.TenantID != nil { normalizedTenantID, err := normalizeRequiredTenantID(filters.TenantID) if err != nil { return 0, err } - filters = &models.BulkDeleteFilters{ - UserID: filters.UserID, - TenantID: &normalizedTenantID, - } + normalizedFilters.TenantID = &normalizedTenantID } - groups, err := s.repo.BulkDelete(ctx, filters) + groups, err := s.repo.DeleteByUser(ctx, normalizedFilters) if err != nil { - return 0, fmt.Errorf("bulk delete feedback records: %w", err) + return 0, fmt.Errorf("delete feedback records by user: %w", err) } deletedCount := 0 @@ -246,7 +255,7 @@ func (s *FeedbackRecordsService) BulkDeleteFeedbackRecords(ctx context.Context, } if group.TenantID == "" { - slog.Error("bulk delete feedback records: deleted rows missing tenant_id; skipping webhook event", + slog.Error("delete feedback records by user: deleted rows missing tenant_id; skipping webhook event", "deleted_count", len(group.IDs), ) diff --git a/internal/service/feedback_records_service_test.go b/internal/service/feedback_records_service_test.go index 13ad699..6e0b282 100644 --- a/internal/service/feedback_records_service_test.go +++ b/internal/service/feedback_records_service_test.go @@ -3,21 +3,23 @@ package service import ( "context" "errors" + "strings" "testing" "time" "github.com/google/uuid" "github.com/formbricks/hub/internal/datatypes" + "github.com/formbricks/hub/internal/huberrors" "github.com/formbricks/hub/internal/models" ) type mockFeedbackRecordsRepo struct { - record *models.FeedbackRecord - createReq *models.CreateFeedbackRecordRequest - bulkGroups []models.DeletedFeedbackRecordsByTenant - deletedID uuid.UUID - bulkDeleteFilters *models.BulkDeleteFilters + record *models.FeedbackRecord + createReq *models.CreateFeedbackRecordRequest + deleteByUserGroups []models.DeletedFeedbackRecordsByTenant + deletedID uuid.UUID + deleteByUserFilters *models.DeleteFeedbackRecordsByUserFilters } func (m *mockFeedbackRecordsRepo) Create( @@ -61,12 +63,12 @@ func (m *mockFeedbackRecordsRepo) Delete(_ context.Context, id uuid.UUID) error return nil } -func (m *mockFeedbackRecordsRepo) BulkDelete( - _ context.Context, filters *models.BulkDeleteFilters, +func (m *mockFeedbackRecordsRepo) DeleteByUser( + _ context.Context, filters *models.DeleteFeedbackRecordsByUserFilters, ) ([]models.DeletedFeedbackRecordsByTenant, error) { - m.bulkDeleteFilters = filters + m.deleteByUserFilters = filters - return m.bulkGroups, nil + return m.deleteByUserGroups, nil } func TestFeedbackRecordsService_DeleteFeedbackRecord_PublishesTenantAwareDeletedEvent(t *testing.T) { @@ -124,14 +126,14 @@ func TestFeedbackRecordsService_CreateFeedbackRecord_NormalizesTenantID(t *testi } } -func TestFeedbackRecordsService_BulkDeleteFeedbackRecords_PublishesTenantAwareDeletedEventsByTenant(t *testing.T) { +func TestFeedbackRecordsService_DeleteFeedbackRecordsByUser_PublishesTenantAwareDeletedEventsByTenant(t *testing.T) { ctx := context.Background() tenantA := "org-123" tenantB := "org-456" tenantAIDs := []uuid.UUID{uuid.Must(uuid.NewV7()), uuid.Must(uuid.NewV7())} tenantBIDs := []uuid.UUID{uuid.Must(uuid.NewV7())} repo := &mockFeedbackRecordsRepo{ - bulkGroups: []models.DeletedFeedbackRecordsByTenant{ + deleteByUserGroups: []models.DeletedFeedbackRecordsByTenant{ {TenantID: tenantA, IDs: tenantAIDs}, {TenantID: tenantB, IDs: tenantBIDs}, }, @@ -139,21 +141,21 @@ func TestFeedbackRecordsService_BulkDeleteFeedbackRecords_PublishesTenantAwareDe publisher := &capturePublisher{} svc := NewFeedbackRecordsService(repo, nil, "", publisher, nil, "", 0) - count, err := svc.BulkDeleteFeedbackRecords(ctx, &models.BulkDeleteFilters{UserID: "user-123"}) + count, err := svc.DeleteFeedbackRecordsByUser(ctx, &models.DeleteFeedbackRecordsByUserFilters{UserID: " user-123 "}) if err != nil { - t.Fatalf("BulkDeleteFeedbackRecords() error = %v", err) + t.Fatalf("DeleteFeedbackRecordsByUser() error = %v", err) } - if repo.bulkDeleteFilters == nil { - t.Fatal("repo BulkDelete filters = nil") + if repo.deleteByUserFilters == nil { + t.Fatal("repo DeleteByUser filters = nil") } - if repo.bulkDeleteFilters.UserID != "user-123" { - t.Fatalf("repo UserID = %q, want user-123", repo.bulkDeleteFilters.UserID) + if repo.deleteByUserFilters.UserID != "user-123" { + t.Fatalf("repo UserID = %q, want user-123", repo.deleteByUserFilters.UserID) } - if repo.bulkDeleteFilters.TenantID != nil { - t.Fatalf("repo TenantID = %q, want nil for all-tenant delete", *repo.bulkDeleteFilters.TenantID) + if repo.deleteByUserFilters.TenantID != nil { + t.Fatalf("repo TenantID = %q, want nil for all-tenant delete", *repo.deleteByUserFilters.TenantID) } if count != len(tenantAIDs)+len(tenantBIDs) { @@ -164,54 +166,107 @@ func TestFeedbackRecordsService_BulkDeleteFeedbackRecords_PublishesTenantAwareDe assertDeletedEventDataAt(t, publisher, 1, datatypes.FeedbackRecordDeleted, tenantB, tenantBIDs) } -func TestFeedbackRecordsService_BulkDeleteFeedbackRecords_NormalizesTenantFilter(t *testing.T) { +func TestFeedbackRecordsService_DeleteFeedbackRecordsByUser_NormalizesTenantFilter(t *testing.T) { ctx := context.Background() tenantID := " org-123 " deletedID := uuid.Must(uuid.NewV7()) repo := &mockFeedbackRecordsRepo{ - bulkGroups: []models.DeletedFeedbackRecordsByTenant{ + deleteByUserGroups: []models.DeletedFeedbackRecordsByTenant{ {TenantID: "org-123", IDs: []uuid.UUID{deletedID}}, }, } publisher := &capturePublisher{} svc := NewFeedbackRecordsService(repo, nil, "", publisher, nil, "", 0) - count, err := svc.BulkDeleteFeedbackRecords(ctx, &models.BulkDeleteFilters{ + count, err := svc.DeleteFeedbackRecordsByUser(ctx, &models.DeleteFeedbackRecordsByUserFilters{ UserID: "user-123", TenantID: &tenantID, }) if err != nil { - t.Fatalf("BulkDeleteFeedbackRecords() error = %v", err) + t.Fatalf("DeleteFeedbackRecordsByUser() error = %v", err) } if count != 1 { t.Fatalf("count = %d, want 1", count) } - if repo.bulkDeleteFilters == nil || repo.bulkDeleteFilters.TenantID == nil { + if repo.deleteByUserFilters == nil || repo.deleteByUserFilters.TenantID == nil { t.Fatal("repo TenantID = nil, want normalized tenant") } - if *repo.bulkDeleteFilters.TenantID != "org-123" { - t.Fatalf("repo TenantID = %q, want org-123", *repo.bulkDeleteFilters.TenantID) + if *repo.deleteByUserFilters.TenantID != "org-123" { + t.Fatalf("repo TenantID = %q, want org-123", *repo.deleteByUserFilters.TenantID) } assertDeletedEventData(t, publisher, datatypes.FeedbackRecordDeleted, "org-123", []uuid.UUID{deletedID}) } -func TestFeedbackRecordsService_BulkDeleteFeedbackRecords_RequiresUserID(t *testing.T) { +func TestFeedbackRecordsService_DeleteFeedbackRecordsByUser_RejectsOverlengthTenantFilter(t *testing.T) { + ctx := context.Background() + tenantID := strings.Repeat("a", maxTenantIDLength+1) + repo := &mockFeedbackRecordsRepo{} + publisher := &capturePublisher{} + svc := NewFeedbackRecordsService(repo, nil, "", publisher, nil, "", 0) + + count, err := svc.DeleteFeedbackRecordsByUser(ctx, &models.DeleteFeedbackRecordsByUserFilters{ + UserID: "user-123", + TenantID: &tenantID, + }) + if !errors.Is(err, huberrors.ErrValidation) { + t.Fatalf("DeleteFeedbackRecordsByUser() error = %v, want validation error", err) + } + + if count != 0 { + t.Fatalf("count = %d, want 0", count) + } + + if repo.deleteByUserFilters != nil { + t.Fatal("repo DeleteByUser was called, want validation before repository") + } + + if publisher.callCount != 0 { + t.Fatalf("published %d events, want 0", publisher.callCount) + } +} + +func TestFeedbackRecordsService_DeleteFeedbackRecordsByUser_RejectsOverlengthUserID(t *testing.T) { + ctx := context.Background() + userID := strings.Repeat("a", maxUserIDLength+1) + repo := &mockFeedbackRecordsRepo{} + publisher := &capturePublisher{} + svc := NewFeedbackRecordsService(repo, nil, "", publisher, nil, "", 0) + + count, err := svc.DeleteFeedbackRecordsByUser(ctx, &models.DeleteFeedbackRecordsByUserFilters{UserID: userID}) + if !errors.Is(err, huberrors.ErrValidation) { + t.Fatalf("DeleteFeedbackRecordsByUser() error = %v, want validation error", err) + } + + if count != 0 { + t.Fatalf("count = %d, want 0", count) + } + + if repo.deleteByUserFilters != nil { + t.Fatal("repo DeleteByUser was called, want validation before repository") + } + + if publisher.callCount != 0 { + t.Fatalf("published %d events, want 0", publisher.callCount) + } +} + +func TestFeedbackRecordsService_DeleteFeedbackRecordsByUser_RequiresUserID(t *testing.T) { ctx := context.Background() repo := &mockFeedbackRecordsRepo{ - bulkGroups: []models.DeletedFeedbackRecordsByTenant{ + deleteByUserGroups: []models.DeletedFeedbackRecordsByTenant{ {TenantID: "org-123", IDs: []uuid.UUID{uuid.Must(uuid.NewV7())}}, }, } publisher := &capturePublisher{} svc := NewFeedbackRecordsService(repo, nil, "", publisher, nil, "", 0) - count, err := svc.BulkDeleteFeedbackRecords(ctx, &models.BulkDeleteFilters{}) + count, err := svc.DeleteFeedbackRecordsByUser(ctx, &models.DeleteFeedbackRecordsByUserFilters{}) if !errors.Is(err, ErrUserIDRequired) { - t.Fatalf("BulkDeleteFeedbackRecords() error = %v, want ErrUserIDRequired", err) + t.Fatalf("DeleteFeedbackRecordsByUser() error = %v, want ErrUserIDRequired", err) } if count != 0 { diff --git a/internal/service/id_validation.go b/internal/service/id_validation.go new file mode 100644 index 0000000..89e729b --- /dev/null +++ b/internal/service/id_validation.go @@ -0,0 +1,48 @@ +package service + +import ( + "strconv" + "strings" + "unicode/utf8" + + "github.com/formbricks/hub/internal/huberrors" +) + +const ( + maxIdentifierLength = 255 + maxTenantIDLength = maxIdentifierLength + maxUserIDLength = maxIdentifierLength +) + +func normalizeRequiredTenantID(tenantID *string) (string, error) { + if tenantID == nil { + return "", huberrors.NewValidationError("tenant_id", "tenant_id is required") + } + + return normalizeRequiredTenantIDValue(*tenantID) +} + +func normalizeRequiredTenantIDValue(tenantID string) (string, error) { + return normalizeRequiredIdentifier("tenant_id", tenantID, maxTenantIDLength) +} + +func normalizeRequiredUserIDValue(userID string) (string, error) { + return normalizeRequiredIdentifier("user_id", userID, maxUserIDLength) +} + +func normalizeRequiredIdentifier(fieldName, value string, maxLength int) (string, error) { + normalized := strings.TrimSpace(value) + if normalized == "" { + return "", huberrors.NewValidationError(fieldName, fieldName+" is required and cannot be empty") + } + + if strings.ContainsRune(normalized, '\x00') { + return "", huberrors.NewValidationError(fieldName, fieldName+" must not contain NULL bytes") + } + + if utf8.RuneCountInString(normalized) > maxLength { + return "", huberrors.NewValidationError(fieldName, fieldName+" must be at most "+strconv.Itoa(maxLength)+" characters") + } + + return normalized, nil +} diff --git a/internal/service/tenant_data_service.go b/internal/service/tenant_data_service.go new file mode 100644 index 0000000..bd57035 --- /dev/null +++ b/internal/service/tenant_data_service.go @@ -0,0 +1,48 @@ +package service + +import ( + "context" + "errors" + "fmt" + + "github.com/formbricks/hub/internal/models" +) + +var errTenantDataNilCounts = errors.New("tenant data repository returned nil counts") + +// TenantDataRepository defines tenant data purge access. +type TenantDataRepository interface { + DeleteByTenant(ctx context.Context, tenantID string) (*models.TenantDataDeleteCounts, error) +} + +// TenantDataService handles tenant data purge business logic. +type TenantDataService struct { + repo TenantDataRepository +} + +// NewTenantDataService creates a new tenant data service. +func NewTenantDataService(repo TenantDataRepository) *TenantDataService { + return &TenantDataService{repo: repo} +} + +// DeleteTenantData deletes all Hub-owned data for a tenant. +func (s *TenantDataService) DeleteTenantData(ctx context.Context, tenantID string) (*models.TenantDataDeleteResult, error) { + normalizedTenantID, err := normalizeRequiredTenantIDValue(tenantID) + if err != nil { + return nil, err + } + + counts, err := s.repo.DeleteByTenant(ctx, normalizedTenantID) + if err != nil { + return nil, fmt.Errorf("delete tenant data: %w", err) + } + + if counts == nil { + return nil, fmt.Errorf("delete tenant data: %w", errTenantDataNilCounts) + } + + return &models.TenantDataDeleteResult{ + TenantID: normalizedTenantID, + TenantDataDeleteCounts: *counts, + }, nil +} diff --git a/internal/service/tenant_data_service_test.go b/internal/service/tenant_data_service_test.go new file mode 100644 index 0000000..bb89ca7 --- /dev/null +++ b/internal/service/tenant_data_service_test.go @@ -0,0 +1,146 @@ +package service + +import ( + "context" + "errors" + "strings" + "testing" + + "github.com/formbricks/hub/internal/huberrors" + "github.com/formbricks/hub/internal/models" +) + +type mockTenantDataRepo struct { + tenantID string + counts *models.TenantDataDeleteCounts + err error +} + +func (m *mockTenantDataRepo) DeleteByTenant( + _ context.Context, tenantID string, +) (*models.TenantDataDeleteCounts, error) { + m.tenantID = tenantID + + return m.counts, m.err +} + +func TestTenantDataService_DeleteTenantData(t *testing.T) { + t.Run("normalizes tenant id and returns counts", func(t *testing.T) { + repo := &mockTenantDataRepo{ + counts: &models.TenantDataDeleteCounts{ + DeletedFeedbackRecords: 4, + DeletedEmbeddings: 3, + DeletedWebhooks: 2, + }, + } + svc := NewTenantDataService(repo) + + result, err := svc.DeleteTenantData(context.Background(), " org-123 ") + if err != nil { + t.Fatalf("DeleteTenantData() error = %v", err) + } + + if repo.tenantID != "org-123" { + t.Fatalf("repo tenantID = %q, want org-123", repo.tenantID) + } + + if result.TenantID != "org-123" { + t.Fatalf("result TenantID = %q, want org-123", result.TenantID) + } + + if result.DeletedFeedbackRecords != 4 || result.DeletedEmbeddings != 3 || result.DeletedWebhooks != 2 { + t.Fatalf("result counts = %+v, want feedback=4 embeddings=3 webhooks=2", result.TenantDataDeleteCounts) + } + }) + + t.Run("rejects invalid tenant id", func(t *testing.T) { + repo := &mockTenantDataRepo{} + svc := NewTenantDataService(repo) + + result, err := svc.DeleteTenantData(context.Background(), " \x00 ") + if !errors.Is(err, huberrors.ErrValidation) { + t.Fatalf("DeleteTenantData() error = %v, want validation", err) + } + + if result != nil { + t.Fatalf("result = %+v, want nil", result) + } + + if repo.tenantID != "" { + t.Fatalf("repo tenantID = %q, want no call", repo.tenantID) + } + }) + + t.Run("wraps repository errors", func(t *testing.T) { + repoErr := errors.New("repository failed") + repo := &mockTenantDataRepo{err: repoErr} + svc := NewTenantDataService(repo) + + result, err := svc.DeleteTenantData(context.Background(), "org-123") + if !errors.Is(err, repoErr) { + t.Fatalf("DeleteTenantData() error = %v, want wrapped repo error", err) + } + + if result != nil { + t.Fatalf("result = %+v, want nil", result) + } + }) + + t.Run("returns error when repository returns nil counts", func(t *testing.T) { + repo := &mockTenantDataRepo{} + svc := NewTenantDataService(repo) + + result, err := svc.DeleteTenantData(context.Background(), "org-123") + if err == nil { + t.Fatal("DeleteTenantData() error = nil, want error") + } + + if !strings.Contains(err.Error(), "repository returned nil counts") { + t.Fatalf("DeleteTenantData() error = %v, want nil counts context", err) + } + + if result != nil { + t.Fatalf("result = %+v, want nil", result) + } + }) +} + +func TestNormalizeRequiredTenantIDValue(t *testing.T) { + longTenantID := make([]rune, maxTenantIDLength+1) + for i := range longTenantID { + longTenantID[i] = 'a' + } + + tests := []struct { + name string + input string + want string + wantErr bool + }{ + {name: "trims whitespace", input: " org-123 ", want: "org-123"}, + {name: "rejects empty", input: " ", wantErr: true}, + {name: "rejects null byte", input: "org-\x00123", wantErr: true}, + {name: "rejects too long", input: string(longTenantID), wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := normalizeRequiredTenantIDValue(tt.input) + if tt.wantErr { + if !errors.Is(err, huberrors.ErrValidation) { + t.Fatalf("normalizeRequiredTenantIDValue() error = %v, want validation", err) + } + + return + } + + if err != nil { + t.Fatalf("normalizeRequiredTenantIDValue() error = %v", err) + } + + if got != tt.want { + t.Fatalf("normalizeRequiredTenantIDValue() = %q, want %q", got, tt.want) + } + }) + } +} diff --git a/internal/service/tenant_validation.go b/internal/service/tenant_validation.go deleted file mode 100644 index 0ecf632..0000000 --- a/internal/service/tenant_validation.go +++ /dev/null @@ -1,24 +0,0 @@ -package service - -import ( - "strings" - - "github.com/formbricks/hub/internal/huberrors" -) - -func normalizeRequiredTenantID(tenantID *string) (string, error) { - if tenantID == nil { - return "", huberrors.NewValidationError("tenant_id", "tenant_id is required") - } - - return normalizeRequiredTenantIDValue(*tenantID) -} - -func normalizeRequiredTenantIDValue(tenantID string) (string, error) { - normalized := strings.TrimSpace(tenantID) - if normalized == "" { - return "", huberrors.NewValidationError("tenant_id", "tenant_id is required and cannot be empty") - } - - return normalized, nil -} diff --git a/openapi.yaml b/openapi.yaml index dcb2513..af5e563 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -44,6 +44,8 @@ tags: description: Feedback record CRUD and search - name: Webhooks description: Webhook subscription management + - name: Tenant Data + description: Tenant-scoped data purge operations security: - ApiKeyAuth: [] paths: @@ -396,7 +398,7 @@ paths: message: "field_type has invalid value \"textt\"; must be one of: text, categorical, nps, csat, ces, rating, number, boolean, date" value: "textt" "409": - description: Conflict – a feedback record with the same tenant_id, submission_id, and field_id already exists (idempotent create). + description: Conflict – duplicate tenant_id/submission_id/field_id. content: application/problem+json: schema: @@ -410,9 +412,14 @@ paths: delete: tags: - Feedback Records - summary: Bulk delete feedback records by user ID - description: Permanently deletes feedback record data points matching the specified user_id. Omit tenant_id to delete that user_id across all tenants for GDPR Article 17 (Right to Erasure) requests. Provide tenant_id to restrict deletion to that tenant only. - operationId: bulk-delete-feedback-records + summary: Delete feedback records by user ID + description: | + Permanently deletes feedback record data points for the specified user_id/data subject. + Omit tenant_id to delete that user_id across all tenants for GDPR Article 17 (Right to Erasure) + requests. Provide tenant_id to restrict deletion to that tenant only. Derived embeddings for deleted + feedback records are removed by database cascade. The operation is idempotent; repeated calls return + deleted_count 0 after matching records have already been deleted. + operationId: delete-feedback-records-by-user parameters: - name: user_id in: query @@ -422,6 +429,7 @@ paths: type: string description: Delete records matching this user ID (required). NULL bytes not allowed. minLength: 1 + maxLength: 255 pattern: '^[^\x00]*$' example: "user-abc-123" - name: tenant_id @@ -431,6 +439,7 @@ paths: type: string description: Optional tenant scope. Omit this parameter to delete all records matching user_id across tenants; provide it to delete only records for this tenant. Empty strings and NULL bytes are not allowed. minLength: 1 + maxLength: 255 pattern: '^[^\x00]*$' example: "org-123" responses: @@ -439,10 +448,10 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/BulkDeleteFeedbackRecordsOutputBody' + $ref: '#/components/schemas/DeleteFeedbackRecordsByUserOutputBody' examples: success: - summary: Successful bulk delete + summary: Successful feedback records delete by user value: deleted_count: 42 message: "Successfully deleted 42 feedback records" @@ -1058,6 +1067,55 @@ paths: application/problem+json: schema: $ref: '#/components/schemas/ErrorModel' + /v1/tenants/{tenant_id}/data: + delete: + tags: + - Tenant Data + summary: Delete tenant data + description: | + Permanently deletes all Hub-owned data for the specified tenant_id. This includes feedback records, + derived embeddings, and webhooks for the tenant. This operation is synchronous and idempotent; + repeated calls return zero counts after the tenant data has already been deleted. + operationId: delete-tenant-data + parameters: + - name: tenant_id + in: path + required: true + description: Tenant ID whose Hub-owned data should be deleted. Empty strings and NULL bytes are not allowed. + schema: + type: string + minLength: 1 + maxLength: 255 + pattern: '^[^\x00]*$' + example: "org-123" + responses: + "200": + description: Tenant data deleted + content: + application/json: + schema: + $ref: '#/components/schemas/TenantDataDeleteOutputBody' + examples: + success: + summary: Successful tenant data delete + value: + tenant_id: "org-123" + deleted_feedback_records: 42 + deleted_embeddings: 17 + deleted_webhooks: 3 + message: "Successfully deleted tenant data for org-123" + "400": + description: Bad Request (e.g. missing or invalid tenant_id) + content: + application/problem+json: + schema: + $ref: '#/components/schemas/ErrorModel' + default: + description: Error + content: + application/problem+json: + schema: + $ref: '#/components/schemas/ErrorModel' components: securitySchemes: ApiKeyAuth: @@ -1066,7 +1124,7 @@ components: bearerFormat: API Key description: API key authentication via Bearer token in Authorization header schemas: - BulkDeleteFeedbackRecordsOutputBody: + DeleteFeedbackRecordsByUserOutputBody: type: object additionalProperties: false properties: @@ -1080,6 +1138,38 @@ components: required: - deleted_count - message + TenantDataDeleteOutputBody: + type: object + additionalProperties: false + properties: + tenant_id: + type: string + description: Tenant ID whose data was deleted + minLength: 1 + maxLength: 255 + pattern: '^[^\x00]*$' + example: "org-123" + deleted_feedback_records: + type: integer + description: Number of feedback records deleted + format: int64 + deleted_embeddings: + type: integer + description: Number of embedding rows deleted + format: int64 + deleted_webhooks: + type: integer + description: Number of webhooks deleted + format: int64 + message: + type: string + description: Human-readable status message + required: + - tenant_id + - deleted_feedback_records + - deleted_embeddings + - deleted_webhooks + - message SemanticSearchInputBody: type: object additionalProperties: false @@ -1642,7 +1732,7 @@ components: type: array description: | For feedback_record.deleted and webhook.deleted events only. - Array of deleted resource IDs (UUID strings). Single delete sends one ID; bulk delete sends all deleted IDs. + Array of deleted resource IDs (UUID strings). Single-resource deletes send one ID; multi-record deletes send all deleted IDs. items: type: string format: uuid diff --git a/tests/README.md b/tests/README.md index c215b7f..974b4fe 100644 --- a/tests/README.md +++ b/tests/README.md @@ -45,7 +45,7 @@ The integration tests cover: - ✅ Get feedback record by ID - ✅ Update feedback record - ✅ Delete feedback record -- ✅ Bulk delete feedback records by user +- ✅ Delete feedback records by user - ✅ Webhook CRUD and validation - ✅ Authentication middleware - ✅ Error handling diff --git a/tests/integration_test.go b/tests/integration_test.go index f14f6ef..72bffd6 100644 --- a/tests/integration_test.go +++ b/tests/integration_test.go @@ -32,8 +32,9 @@ import ( const defaultTestDatabaseURL = "postgres://postgres:postgres@localhost:5432/test_db?sslmode=disable" const ( - testWebhookURL = "https://192.0.2.1/webhook" - testWebhookURLV2 = "https://192.0.2.1/webhook-v2" + tenantDataCleanupTimeout = 2 * time.Second + testWebhookURL = "https://192.0.2.1/webhook" + testWebhookURLV2 = "https://192.0.2.1/webhook-v2" ) // requireUUIDv7 asserts that an ID uses UUID version 7 with the RFC4122 variant. @@ -80,6 +81,7 @@ func setupTestServer(t *testing.T) (server *httptest.Server, cleanup func()) { // Initialize repository, service, and handler layers feedbackRecordsRepo := repository.NewFeedbackRecordsRepository(db) embeddingsRepo := repository.NewEmbeddingsRepository(db) + tenantDataRepo := repository.NewTenantDataRepository(db) feedbackRecordsService := service.NewFeedbackRecordsService( feedbackRecordsRepo, embeddingsRepo, @@ -90,6 +92,8 @@ func setupTestServer(t *testing.T) (server *httptest.Server, cleanup func()) { 0, ) feedbackRecordsHandler := handlers.NewFeedbackRecordsHandler(feedbackRecordsService) + tenantDataService := service.NewTenantDataService(tenantDataRepo) + tenantDataHandler := handlers.NewTenantDataHandler(tenantDataService) healthHandler := handlers.NewHealthHandler() // Set up public endpoints @@ -105,12 +109,13 @@ func setupTestServer(t *testing.T) (server *httptest.Server, cleanup func()) { protectedMux.HandleFunc("GET /v1/feedback-records/{id}", feedbackRecordsHandler.Get) protectedMux.HandleFunc("PATCH /v1/feedback-records/{id}", feedbackRecordsHandler.Update) protectedMux.HandleFunc("DELETE /v1/feedback-records/{id}", feedbackRecordsHandler.Delete) - protectedMux.HandleFunc("DELETE /v1/feedback-records", feedbackRecordsHandler.BulkDelete) + protectedMux.HandleFunc("DELETE /v1/feedback-records", feedbackRecordsHandler.DeleteByUser) protectedMux.HandleFunc("POST /v1/webhooks", webhooksHandler.Create) protectedMux.HandleFunc("GET /v1/webhooks", webhooksHandler.List) protectedMux.HandleFunc("GET /v1/webhooks/{id}", webhooksHandler.Get) protectedMux.HandleFunc("PATCH /v1/webhooks/{id}", webhooksHandler.Update) protectedMux.HandleFunc("DELETE /v1/webhooks/{id}", webhooksHandler.Delete) + protectedMux.HandleFunc("DELETE /v1/tenants/{tenant_id}/data", tenantDataHandler.Delete) var protectedHandler http.Handler = protectedMux @@ -880,12 +885,12 @@ func TestDeleteFeedbackRecord(t *testing.T) { }) } -func TestBulkDeleteFeedbackRecords(t *testing.T) { +func TestDeleteFeedbackRecordsByUser(t *testing.T) { server, cleanup := setupTestServer(t) defer cleanup() client := &http.Client{} - userID := "bulk-delete-test-user-" + uuid.New().String() + userID := "user-delete-test-user-" + uuid.New().String() subID := uuid.New().String() // unique per run to avoid 409 from leftover data // Create several feedback records with the same user_id across tenants. @@ -952,7 +957,7 @@ func TestBulkDeleteFeedbackRecords(t *testing.T) { require.NoError(t, err) assert.Equal(t, http.StatusOK, resp.StatusCode) - var scopedResp models.BulkDeleteResponse + var scopedResp models.DeleteFeedbackRecordsByUserResponse err = decodeData(resp, &scopedResp) require.NoError(t, err) @@ -966,8 +971,8 @@ func TestBulkDeleteFeedbackRecords(t *testing.T) { tenantAID2 := createRecord("nps_4", tenantA, 7) // Omitting tenant_id deletes every remaining matching record for GDPR erasure, regardless of tenant. - bulkDelURL := server.URL + "/v1/feedback-records?user_id=" + url.QueryEscape(userID) - req, err = http.NewRequestWithContext(context.Background(), http.MethodDelete, bulkDelURL, http.NoBody) + userDeleteURL := server.URL + "/v1/feedback-records?user_id=" + url.QueryEscape(userID) + req, err = http.NewRequestWithContext(context.Background(), http.MethodDelete, userDeleteURL, http.NoBody) require.NoError(t, err) req.Header.Set("Authorization", "Bearer "+testAPIKey) @@ -975,38 +980,243 @@ func TestBulkDeleteFeedbackRecords(t *testing.T) { require.NoError(t, err) assert.Equal(t, http.StatusOK, resp.StatusCode) - var bulkResp models.BulkDeleteResponse + var userDeleteResp models.DeleteFeedbackRecordsByUserResponse - err = decodeData(resp, &bulkResp) + err = decodeData(resp, &userDeleteResp) require.NoError(t, err) require.NoError(t, resp.Body.Close()) - assert.Equal(t, int64(3), bulkResp.DeletedCount) - assert.Equal(t, "Successfully deleted 3 feedback records", bulkResp.Message) + assert.Equal(t, int64(3), userDeleteResp.DeletedCount) + assert.Equal(t, "Successfully deleted 3 feedback records", userDeleteResp.Message) // Verify records are gone for _, id := range []string{tenantAID2, tenantBID1, tenantBID2} { requireStatus(id, http.StatusNotFound) } - // Bulk delete again (no matching records) returns 0 - bulkDelURL2 := server.URL + "/v1/feedback-records?user_id=" + url.QueryEscape(userID) - req2, err := http.NewRequestWithContext(context.Background(), http.MethodDelete, bulkDelURL2, http.NoBody) + // Deleting again with no matching records returns 0. + userDeleteURL2 := server.URL + "/v1/feedback-records?user_id=" + url.QueryEscape(userID) + req2, err := http.NewRequestWithContext(context.Background(), http.MethodDelete, userDeleteURL2, http.NoBody) require.NoError(t, err) req2.Header.Set("Authorization", "Bearer "+testAPIKey) resp2, err := client.Do(req2) require.NoError(t, err) assert.Equal(t, http.StatusOK, resp2.StatusCode) - var bulkResp2 models.BulkDeleteResponse + var userDeleteResp2 models.DeleteFeedbackRecordsByUserResponse - err = decodeData(resp2, &bulkResp2) + err = decodeData(resp2, &userDeleteResp2) require.NoError(t, err) require.NoError(t, resp2.Body.Close()) - assert.Equal(t, int64(0), bulkResp2.DeletedCount) + assert.Equal(t, int64(0), userDeleteResp2.DeletedCount) } -// TestFeedbackRecordsRepository_BulkDelete tests the repository BulkDelete return value (deleted IDs). -func TestFeedbackRecordsRepository_BulkDelete(t *testing.T) { +func TestDeleteTenantData(t *testing.T) { + server, cleanup := setupTestServer(t) + defer cleanup() + + ctx := context.Background() + cfg, err := config.Load() + require.NoError(t, err) + db, err := database.NewPostgresPool(ctx, cfg.Database.URL, + database.WithPoolConfig(cfg.Database.PoolConfig()), + ) + require.NoError(t, err) + + defer db.Close() + + embeddingsRepo := repository.NewEmbeddingsRepository(db) + client := &http.Client{} + tenantA := "tenant-data-delete-a-" + uuid.NewString() + tenantB := "tenant-data-delete-b-" + uuid.NewString() + subID := uuid.NewString() + modelName := "model-name" + + tenantARecord1 := createTenantDataFeedbackRecord(ctx, t, client, server.URL, tenantA, subID, "tenant-data-delete-a-1") + tenantARecord2 := createTenantDataFeedbackRecord(ctx, t, client, server.URL, tenantA, subID, "tenant-data-delete-a-2") + tenantBRecord := createTenantDataFeedbackRecord(ctx, t, client, server.URL, tenantB, subID, "tenant-data-delete-b-1") + tenantAWebhook := createTenantDataWebhook(ctx, t, client, server.URL, tenantA, "tenant-data-delete-a") + + tenantBWebhook := createTenantDataWebhook(ctx, t, client, server.URL, tenantB, "tenant-data-delete-b") + defer cleanupTenantDataBestEffort(ctx, client, server.URL, tenantB) + + embedding := make([]float32, models.EmbeddingVectorDimensions) + embedding[0] = 0.25 + require.NoError(t, embeddingsRepo.Upsert(ctx, tenantARecord1.ID, modelName, embedding)) + require.NoError(t, embeddingsRepo.Upsert(ctx, tenantARecord2.ID, modelName, embedding)) + require.NoError(t, embeddingsRepo.Upsert(ctx, tenantBRecord.ID, modelName, embedding)) + + deleteResp := deleteTenantData(ctx, t, client, server.URL, tenantA) + assert.Equal(t, tenantA, deleteResp.TenantID) + assert.Equal(t, int64(2), deleteResp.DeletedFeedbackRecords) + assert.Equal(t, int64(2), deleteResp.DeletedEmbeddings) + assert.Equal(t, int64(1), deleteResp.DeletedWebhooks) + + requireTenantDataResourceStatus( + ctx, t, client, fmt.Sprintf("%s/v1/feedback-records/%s", server.URL, tenantARecord1.ID), http.StatusNotFound, + ) + requireTenantDataResourceStatus( + ctx, t, client, fmt.Sprintf("%s/v1/feedback-records/%s", server.URL, tenantARecord2.ID), http.StatusNotFound, + ) + requireTenantDataResourceStatus( + ctx, t, client, fmt.Sprintf("%s/v1/webhooks/%s", server.URL, tenantAWebhook.ID), http.StatusNotFound, + ) + _, err = embeddingsRepo.GetEmbeddingByFeedbackRecordAndModel(ctx, tenantARecord1.ID, modelName) + require.ErrorIs(t, err, repository.ErrEmbeddingNotFound) + + requireTenantDataResourceStatus( + ctx, t, client, fmt.Sprintf("%s/v1/feedback-records/%s", server.URL, tenantBRecord.ID), http.StatusOK, + ) + requireTenantDataResourceStatus(ctx, t, client, fmt.Sprintf("%s/v1/webhooks/%s", server.URL, tenantBWebhook.ID), http.StatusOK) + _, err = embeddingsRepo.GetEmbeddingByFeedbackRecordAndModel(ctx, tenantBRecord.ID, modelName) + require.NoError(t, err) + + repeatedResp := deleteTenantData(ctx, t, client, server.URL, tenantA) + assert.Equal(t, int64(0), repeatedResp.DeletedFeedbackRecords) + assert.Equal(t, int64(0), repeatedResp.DeletedEmbeddings) + assert.Equal(t, int64(0), repeatedResp.DeletedWebhooks) +} + +func createTenantDataFeedbackRecord( + ctx context.Context, + t *testing.T, + client *http.Client, + serverURL string, + tenantID string, + submissionID string, + fieldID string, +) models.FeedbackRecord { + t.Helper() + + body, err := json.Marshal(map[string]any{ + "source_type": "formbricks", + "submission_id": submissionID, + "tenant_id": tenantID, + "user_id": "tenant-data-delete-user-" + uuid.NewString(), + "field_id": fieldID, + "field_type": "text", + "value_text": "delete tenant data", + }) + require.NoError(t, err) + + req, err := http.NewRequestWithContext(ctx, http.MethodPost, serverURL+"/v1/feedback-records", bytes.NewBuffer(body)) + require.NoError(t, err) + req.Header.Set("Authorization", "Bearer "+testAPIKey) + req.Header.Set("Content-Type", "application/json") + + resp, err := client.Do(req) + require.NoError(t, err) + require.Equal(t, http.StatusCreated, resp.StatusCode) + + var rec models.FeedbackRecord + + err = decodeData(resp, &rec) + require.NoError(t, err) + require.NoError(t, resp.Body.Close()) + + return rec +} + +func createTenantDataWebhook( + ctx context.Context, + t *testing.T, + client *http.Client, + serverURL string, + tenantID string, + path string, +) models.Webhook { + t.Helper() + + body, err := json.Marshal(map[string]any{ + "url": testWebhookURL + "/" + path, + "tenant_id": tenantID, + }) + require.NoError(t, err) + + req, err := http.NewRequestWithContext(ctx, http.MethodPost, serverURL+"/v1/webhooks", bytes.NewBuffer(body)) + require.NoError(t, err) + req.Header.Set("Authorization", "Bearer "+testAPIKey) + req.Header.Set("Content-Type", "application/json") + + resp, err := client.Do(req) + require.NoError(t, err) + require.Equal(t, http.StatusCreated, resp.StatusCode) + + var webhook models.Webhook + + err = decodeData(resp, &webhook) + require.NoError(t, err) + require.NoError(t, resp.Body.Close()) + + return webhook +} + +func requireTenantDataResourceStatus( + ctx context.Context, + t *testing.T, + client *http.Client, + resourceURL string, + status int, +) { + t.Helper() + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, resourceURL, http.NoBody) + require.NoError(t, err) + req.Header.Set("Authorization", "Bearer "+testAPIKey) + + resp, err := client.Do(req) + require.NoError(t, err) + assert.Equal(t, status, resp.StatusCode) + require.NoError(t, resp.Body.Close()) +} + +func deleteTenantData( + ctx context.Context, + t *testing.T, + client *http.Client, + serverURL string, + tenantID string, +) models.TenantDataDeleteResponse { + t.Helper() + + deleteURL := fmt.Sprintf("%s/v1/tenants/%s/data", serverURL, url.PathEscape(tenantID)) + req, err := http.NewRequestWithContext(ctx, http.MethodDelete, deleteURL, http.NoBody) + require.NoError(t, err) + req.Header.Set("Authorization", "Bearer "+testAPIKey) + + resp, err := client.Do(req) + require.NoError(t, err) + assert.Equal(t, http.StatusOK, resp.StatusCode) + + var deleteResp models.TenantDataDeleteResponse + + err = decodeData(resp, &deleteResp) + require.NoError(t, err) + require.NoError(t, resp.Body.Close()) + + return deleteResp +} + +func cleanupTenantDataBestEffort(ctx context.Context, client *http.Client, serverURL, tenantID string) { + cleanupCtx, cancel := context.WithTimeout(ctx, tenantDataCleanupTimeout) + defer cancel() + + cleanupURL := fmt.Sprintf("%s/v1/tenants/%s/data", serverURL, url.PathEscape(tenantID)) + + req, err := http.NewRequestWithContext(cleanupCtx, http.MethodDelete, cleanupURL, http.NoBody) + if err != nil { + return + } + + req.Header.Set("Authorization", "Bearer "+testAPIKey) + + resp, err := client.Do(req) + if err == nil { + _ = resp.Body.Close() + } +} + +// TestFeedbackRecordsRepository_DeleteByUser tests the repository DeleteByUser return value (deleted IDs). +func TestFeedbackRecordsRepository_DeleteByUser(t *testing.T) { ctx := context.Background() databaseURL := os.Getenv("DATABASE_URL") @@ -1027,23 +1237,24 @@ func TestFeedbackRecordsRepository_BulkDelete(t *testing.T) { defer db.Close() repo := repository.NewFeedbackRecordsRepository(db) - userID := "repo-bulk-delete-user-" + uuid.New().String() + userID := "repo-user-delete-user-" + uuid.New().String() sourceType := "formbricks" // Create records with same user_id across tenants. - const bulkDeleteTenant = "bulk-delete-tenant" + const deleteByUserTenant = "user-delete-tenant" - const otherBulkDeleteTenant = "bulk-delete-tenant-other" + const otherDeleteByUserTenant = "user-delete-tenant-other" req1 := &models.CreateFeedbackRecordRequest{ SourceType: sourceType, SubmissionID: userID, - TenantID: bulkDeleteTenant, + TenantID: deleteByUserTenant, FieldID: "f1", FieldType: models.FieldTypeNumber, - ValueNumber: new(1.0), UserID: &userID, } + valueNumber1 := 1.0 + req1.ValueNumber = &valueNumber1 rec1, err := repo.Create(ctx, req1) require.NoError(t, err) require.NotEmpty(t, rec1.ID) @@ -1051,12 +1262,13 @@ func TestFeedbackRecordsRepository_BulkDelete(t *testing.T) { req2 := &models.CreateFeedbackRecordRequest{ SourceType: sourceType, SubmissionID: userID, - TenantID: bulkDeleteTenant, + TenantID: deleteByUserTenant, FieldID: "f2", FieldType: models.FieldTypeNumber, - ValueNumber: new(2.0), UserID: &userID, } + valueNumber2 := 2.0 + req2.ValueNumber = &valueNumber2 rec2, err := repo.Create(ctx, req2) require.NoError(t, err) require.NotEmpty(t, rec2.ID) @@ -1065,7 +1277,7 @@ func TestFeedbackRecordsRepository_BulkDelete(t *testing.T) { req3 := &models.CreateFeedbackRecordRequest{ SourceType: sourceType, SubmissionID: userID, - TenantID: otherBulkDeleteTenant, + TenantID: otherDeleteByUserTenant, FieldID: "f3", FieldType: models.FieldTypeText, ValueText: &valueText, @@ -1075,15 +1287,15 @@ func TestFeedbackRecordsRepository_BulkDelete(t *testing.T) { require.NoError(t, err) require.NotEmpty(t, rec3.ID) - // BulkDelete with tenant_id restricts deletion to that tenant and returns tenant-safe groups. - tenantFilter := bulkDeleteTenant - deletedGroups, err := repo.BulkDelete(ctx, &models.BulkDeleteFilters{ + // DeleteByUser with tenant_id restricts deletion to that tenant and returns tenant-safe groups. + tenantFilter := deleteByUserTenant + deletedGroups, err := repo.DeleteByUser(ctx, &models.DeleteFeedbackRecordsByUserFilters{ UserID: userID, TenantID: &tenantFilter, }) require.NoError(t, err) require.Len(t, deletedGroups, 1) - require.Equal(t, bulkDeleteTenant, deletedGroups[0].TenantID) + require.Equal(t, deleteByUserTenant, deletedGroups[0].TenantID) assert.ElementsMatch(t, []uuid.UUID{rec1.ID, rec2.ID}, deletedGroups[0].IDs) _, err = repo.GetByID(ctx, rec1.ID) @@ -1092,13 +1304,13 @@ func TestFeedbackRecordsRepository_BulkDelete(t *testing.T) { require.Error(t, err) remaining, err := repo.GetByID(ctx, rec3.ID) require.NoError(t, err) - require.Equal(t, otherBulkDeleteTenant, remaining.TenantID) + require.Equal(t, otherDeleteByUserTenant, remaining.TenantID) // Omitting tenant_id deletes the rest of the user records across tenants. - deletedGroups, err = repo.BulkDelete(ctx, &models.BulkDeleteFilters{UserID: userID}) + deletedGroups, err = repo.DeleteByUser(ctx, &models.DeleteFeedbackRecordsByUserFilters{UserID: userID}) require.NoError(t, err) require.Len(t, deletedGroups, 1) - require.Equal(t, otherBulkDeleteTenant, deletedGroups[0].TenantID) + require.Equal(t, otherDeleteByUserTenant, deletedGroups[0].TenantID) assert.ElementsMatch(t, []uuid.UUID{rec3.ID}, deletedGroups[0].IDs) _, err = repo.GetByID(ctx, rec3.ID) From 698efd340cfd888313d5018827fad8953a057a5f Mon Sep 17 00:00:00 2001 From: Tiago Farto Date: Wed, 20 May 2026 13:13:48 +0000 Subject: [PATCH 2/5] chore: address PR concerns --- Makefile | 18 +++--- cmd/api/app_test.go | 18 ++++++ .../api/handlers/feedback_records_handler.go | 10 +-- internal/api/handlers/tenant_data_handler.go | 8 ++- internal/repository/tenant_data_repository.go | 14 ++++- .../repository/tenant_data_repository_test.go | 2 + internal/repository/webhooks_repository.go | 6 +- .../repository/webhooks_repository_test.go | 41 ++++++++++++ openapi.yaml | 1 + tests/README.md | 1 + tests/integration_test.go | 62 ++++++++++++++++++- 11 files changed, 161 insertions(+), 20 deletions(-) diff --git a/Makefile b/Makefile index dc30013..517f59e 100644 --- a/Makefile +++ b/Makefile @@ -22,12 +22,12 @@ help: @echo " make mcp-smoke - Run the live MCP package smoke test (requires Hub env vars)" @echo " make test-all - Run all tests (unit + integration)" @echo " make tests-coverage - Run tests with coverage report" - @echo " make check-coverage - Run tests and fail if coverage below COVERAGE_THRESHOLD (excludes cmd/api)" + @echo " make check-coverage - Run tests and fail if coverage below COVERAGE_THRESHOLD" @echo " make init-db - Initialize database schema (run migrations with goose)" @echo " make migrate-status - Show migration status" @echo " make migrate-validate - Validate migration files (no DB)" @echo " make river-migrate - Run River job queue migrations (required for webhook delivery)" - @echo " make fmt - Format code (golangci-lint run --fix)" + @echo " make fmt - Format code" @echo " make lint - Run linter (includes format checks)" @echo " make lint-new - Run linter only on new code since base (default origin/main; for CI set LINT_BASE_REV to PR base SHA)" @echo " make deps - Install Go dependencies" @@ -66,11 +66,11 @@ test-unit: test-all: test-unit tests @echo "All tests passed!" -# Run tests with coverage (unit + integration). -# cmd/api (app.go, main.go) is excluded—coverage is for internal/, pkg/, and tests/. +# Run tests with package-instrumented coverage (unit + integration). +# Integration tests live in tests/, so coverpkg is required for those tests to count against app packages. tests-coverage: @echo "Running tests with coverage..." - go test ./internal/... ./pkg/... ./tests/... -v -cover -coverprofile=coverage.out + @(set -a && [ -f .env ] && . ./.env && set +a; go test ./cmd/api ./internal/... ./pkg/... ./tests/... -v -coverpkg=./cmd/api,./internal/...,./pkg/... -coverprofile=coverage.out) go tool cover -html=coverage.out -o coverage.html @echo "Coverage report generated: coverage.html" @@ -78,10 +78,10 @@ tests-coverage: COVERAGE_THRESHOLD ?= 15 # Check coverage threshold (fail if below COVERAGE_THRESHOLD). -# Excludes cmd/api (app.go, main.go) from coverage. Includes internal/, tests/, and pkg/. +# Uses package-instrumented coverage so integration tests contribute to app package coverage. check-coverage: @echo "Running tests with coverage (threshold: $(COVERAGE_THRESHOLD)%)..." - @(set -a && [ -f .env ] && . ./.env && set +a; go test ./internal/... ./pkg/... ./tests/... -coverprofile=coverage.out) + @(set -a && [ -f .env ] && . ./.env && set +a; go test ./cmd/api ./internal/... ./pkg/... ./tests/... -coverpkg=./cmd/api,./internal/...,./pkg/... -coverprofile=coverage.out) @COV=$$(go tool cover -func=coverage.out | \tail -1 | awk '{gsub(/%/, ""); print $$3}') && \ if [ -z "$$COV" ] || ! awk -v c="$$COV" -v t="$(COVERAGE_THRESHOLD)" 'BEGIN { exit (c+0 >= t) ? 0 : 1 }'; then \ echo ""; \ @@ -292,11 +292,11 @@ install-tools: go install github.com/riverqueue/river/cmd/river@$(RIVER_VERSION) @echo "Tools installed (golangci-lint $(GOLANGCI_LINT_VERSION), govulncheck $(GOVULNCHECK_VERSION), goose $(GOOSE_VERSION), river $(RIVER_VERSION))" -# Format code (golangci-lint applies gofumpt + gci from .golangci.yml formatters) +# Format code (golangci-lint applies gofumpt + gci from .golangci.yml formatters). fmt: @echo "Formatting code..." @test -x $(GOLANGCI_LINT) || { echo "Error: golangci-lint not found. Install with: make install-tools"; exit 1; } - $(GOLANGCI_LINT) run --fix ./... + $(GOLANGCI_LINT) fmt ./... @echo "Code formatted" # Lint code diff --git a/cmd/api/app_test.go b/cmd/api/app_test.go index 7a2c117..984e1b7 100644 --- a/cmd/api/app_test.go +++ b/cmd/api/app_test.go @@ -240,6 +240,24 @@ func TestNewHTTPServerKeepsV1RoutesProtected(t *testing.T) { } } +func TestNewHTTPServerKeepsTenantDataRoutesProtected(t *testing.T) { + server := newTestHTTPServer(t) + + recorder := httptest.NewRecorder() + request := httptest.NewRequestWithContext( + context.Background(), + http.MethodDelete, + "/v1/tenants/test-tenant-id/data", + http.NoBody, + ) + + server.Handler.ServeHTTP(recorder, request) + + if recorder.Code != http.StatusUnauthorized { + t.Fatalf("DELETE /v1/tenants/{tenant_id}/data status = %d, want %d", recorder.Code, http.StatusUnauthorized) + } +} + func newTestHTTPServer(t *testing.T) *http.Server { t.Helper() diff --git a/internal/api/handlers/feedback_records_handler.go b/internal/api/handlers/feedback_records_handler.go index d016fe3..0f2a010 100644 --- a/internal/api/handlers/feedback_records_handler.go +++ b/internal/api/handlers/feedback_records_handler.go @@ -245,16 +245,18 @@ func (h *FeedbackRecordsHandler) DeleteByUser(w http.ResponseWriter, r *http.Req return } - var tenantID string + tenantIDLength := 0 if filters.TenantID != nil { - tenantID = *filters.TenantID + tenantIDLength = len(*filters.TenantID) } slog.Error("Failed to delete feedback records by user", // #nosec G706 -- slog key-values "method", r.Method, "path", r.URL.Path, - "user_id", filters.UserID, - "tenant_id", tenantID, + "user_id_present", filters.UserID != "", + "user_id_length", len(filters.UserID), + "tenant_id_present", tenantIDLength > 0, + "tenant_id_length", tenantIDLength, "error", err, ) diff --git a/internal/api/handlers/tenant_data_handler.go b/internal/api/handlers/tenant_data_handler.go index 5e5cdbc..cc9ef25 100644 --- a/internal/api/handlers/tenant_data_handler.go +++ b/internal/api/handlers/tenant_data_handler.go @@ -12,6 +12,8 @@ import ( "github.com/formbricks/hub/internal/models" ) +const tenantDataDeleteRoute = "DELETE /v1/tenants/{tenant_id}/data" + // TenantDataService defines the interface for tenant data purge business logic. type TenantDataService interface { DeleteTenantData(ctx context.Context, tenantID string) (*models.TenantDataDeleteResult, error) @@ -40,9 +42,9 @@ func (h *TenantDataHandler) Delete(w http.ResponseWriter, r *http.Request) { } slog.Error("Failed to delete tenant data", // #nosec G706 -- slog key-values - "method", r.Method, - "path", r.URL.Path, - "tenant_id", tenantID, + "route", tenantDataDeleteRoute, + "tenant_id_present", tenantID != "", + "tenant_id_length", len(tenantID), "error", err, ) diff --git a/internal/repository/tenant_data_repository.go b/internal/repository/tenant_data_repository.go index 2945177..ccf615f 100644 --- a/internal/repository/tenant_data_repository.go +++ b/internal/repository/tenant_data_repository.go @@ -59,7 +59,12 @@ func (r *TenantDataRepository) DeleteByTenant(ctx context.Context, tenantID stri defer func() { if err := dbTx.Rollback(ctx); err != nil && !errors.Is(err, pgx.ErrTxClosed) { - slog.Error("tenant data delete: rollback failed", "tenant_id", tenantID, "error", err) + slog.Error( + "tenant data delete: rollback failed", + "tenant_id_present", tenantID != "", + "tenant_id_length", len(tenantID), + "error", err, + ) } }() @@ -69,7 +74,12 @@ func (r *TenantDataRepository) DeleteByTenant(ctx context.Context, tenantID stri } if err := dbTx.Commit(ctx); err != nil { - slog.Error("tenant data delete: commit failed", "tenant_id", tenantID, "error", err) + slog.Error( + "tenant data delete: commit failed", + "tenant_id_present", tenantID != "", + "tenant_id_length", len(tenantID), + "error", err, + ) return nil, fmt.Errorf("commit tenant data delete transaction: %w", err) } diff --git a/internal/repository/tenant_data_repository_test.go b/internal/repository/tenant_data_repository_test.go index 8e38436..c705140 100644 --- a/internal/repository/tenant_data_repository_test.go +++ b/internal/repository/tenant_data_repository_test.go @@ -107,8 +107,10 @@ func TestTenantDataRepository_DeleteByTenant(t *testing.T) { }) t.Run("rolls back and returns delete error", func(t *testing.T) { + rollbackErr := errors.New("rollback failed") transaction := &fakeTenantDataTx{ fakeTenantDataExecutor: fakeTenantDataExecutor{errAtQuery: 2}, + rollbackErr: rollbackErr, } repo := &TenantDataRepository{db: &fakeTenantDataDB{tx: transaction}} diff --git a/internal/repository/webhooks_repository.go b/internal/repository/webhooks_repository.go index e6baaad..bd6fc97 100644 --- a/internal/repository/webhooks_repository.go +++ b/internal/repository/webhooks_repository.go @@ -28,7 +28,11 @@ func NewWebhooksRepository(db *pgxpool.Pool) *WebhooksRepository { // Create inserts a new webhook. func (r *WebhooksRepository) Create(ctx context.Context, req *models.CreateWebhookRequest) (*models.Webhook, error) { - if req.TenantID == nil { + if req == nil { + return nil, huberrors.NewValidationError("request", "request is required") + } + + if req.TenantID == nil || strings.TrimSpace(*req.TenantID) == "" { return nil, huberrors.NewValidationError("tenant_id", "tenant_id is required") } diff --git a/internal/repository/webhooks_repository_test.go b/internal/repository/webhooks_repository_test.go index 5ea7704..b57dc9c 100644 --- a/internal/repository/webhooks_repository_test.go +++ b/internal/repository/webhooks_repository_test.go @@ -1,6 +1,7 @@ package repository import ( + "context" "strings" "testing" @@ -8,6 +9,8 @@ import ( "github.com/stretchr/testify/require" "github.com/formbricks/hub/internal/datatypes" + "github.com/formbricks/hub/internal/huberrors" + "github.com/formbricks/hub/internal/models" ) func TestListEnabledForEventTypeAndTenantQuery(t *testing.T) { @@ -49,3 +52,41 @@ func TestListEnabledForEventTypeAndTenantQuery(t *testing.T) { }) } } + +func TestWebhooksRepository_CreateValidatesTenantBoundary(t *testing.T) { + emptyTenantID := "" + whitespaceTenantID := " \t\n " + + tests := []struct { + name string + req *models.CreateWebhookRequest + }{ + { + name: "nil request", + }, + { + name: "nil tenant", + req: &models.CreateWebhookRequest{}, + }, + { + name: "empty tenant", + req: &models.CreateWebhookRequest{TenantID: &emptyTenantID}, + }, + { + name: "whitespace tenant", + req: &models.CreateWebhookRequest{TenantID: &whitespaceTenantID}, + }, + } + + repo := NewWebhooksRepository(nil) + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + webhook, err := repo.Create(context.Background(), tt.req) + + require.Nil(t, webhook) + require.Error(t, err) + assert.ErrorIs(t, err, huberrors.ErrValidation) + }) + } +} diff --git a/openapi.yaml b/openapi.yaml index af5e563..4db06c0 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -1076,6 +1076,7 @@ paths: Permanently deletes all Hub-owned data for the specified tenant_id. This includes feedback records, derived embeddings, and webhooks for the tenant. This operation is synchronous and idempotent; repeated calls return zero counts after the tenant data has already been deleted. + No webhook events are published as part of this purge operation. operationId: delete-tenant-data parameters: - name: tenant_id diff --git a/tests/README.md b/tests/README.md index 974b4fe..0e4f039 100644 --- a/tests/README.md +++ b/tests/README.md @@ -46,6 +46,7 @@ The integration tests cover: - ✅ Update feedback record - ✅ Delete feedback record - ✅ Delete feedback records by user +- ✅ Delete tenant data (feedback records, embeddings, webhooks) - ✅ Webhook CRUD and validation - ✅ Authentication middleware - ✅ Error handling diff --git a/tests/integration_test.go b/tests/integration_test.go index 72bffd6..89021d3 100644 --- a/tests/integration_test.go +++ b/tests/integration_test.go @@ -10,6 +10,7 @@ import ( "net/http/httptest" "net/url" "os" + "sync" "testing" "time" @@ -21,12 +22,17 @@ import ( "github.com/formbricks/hub/internal/api/middleware" "github.com/formbricks/hub/internal/api/response" "github.com/formbricks/hub/internal/config" + "github.com/formbricks/hub/internal/datatypes" "github.com/formbricks/hub/internal/models" "github.com/formbricks/hub/internal/repository" "github.com/formbricks/hub/internal/service" "github.com/formbricks/hub/pkg/database" ) +type testEventProvider interface { + PublishEvent(ctx context.Context, event service.Event) +} + // defaultTestDatabaseURL is the default Postgres URL used by compose (postgres/postgres/test_db). // Used when DATABASE_URL is not set (e.g. CI uses job env; local can rely on .env). const defaultTestDatabaseURL = "postgres://postgres:postgres@localhost:5432/test_db?sslmode=disable" @@ -49,6 +55,12 @@ func requireUUIDv7(t *testing.T, id uuid.UUID) { // setupTestServer creates a test HTTP server with all routes configured. // Database URL comes from env (DATABASE_URL) when set; otherwise config.Load() uses its default. func setupTestServer(t *testing.T) (server *httptest.Server, cleanup func()) { + return setupTestServerWithEventProviders(t) +} + +func setupTestServerWithEventProviders( + t *testing.T, providers ...testEventProvider, +) (server *httptest.Server, cleanup func()) { ctx := context.Background() databaseURL := os.Getenv("DATABASE_URL") @@ -71,7 +83,11 @@ func setupTestServer(t *testing.T) (server *httptest.Server, cleanup func()) { // Initialize message publisher manager for tests (no providers required) perEventTimeout := time.Duration(cfg.MessagePublisher.PerEventTimeoutSec) * time.Second + messageManager := service.NewMessagePublisherManager(cfg.MessagePublisher.BufferSize, perEventTimeout, nil) + for _, provider := range providers { + messageManager.RegisterProvider(provider) + } // Webhooks webhooksRepo := repository.NewWebhooksRepository(db) @@ -1011,7 +1027,9 @@ func TestDeleteFeedbackRecordsByUser(t *testing.T) { } func TestDeleteTenantData(t *testing.T) { - server, cleanup := setupTestServer(t) + eventRecorder := &tenantDataEventRecorder{} + + server, cleanup := setupTestServerWithEventProviders(t, eventRecorder) defer cleanup() ctx := context.Background() @@ -1050,6 +1068,7 @@ func TestDeleteTenantData(t *testing.T) { assert.Equal(t, int64(2), deleteResp.DeletedFeedbackRecords) assert.Equal(t, int64(2), deleteResp.DeletedEmbeddings) assert.Equal(t, int64(1), deleteResp.DeletedWebhooks) + requireNoTenantDataDeleteEvents(t, eventRecorder) requireTenantDataResourceStatus( ctx, t, client, fmt.Sprintf("%s/v1/feedback-records/%s", server.URL, tenantARecord1.ID), http.StatusNotFound, @@ -1074,6 +1093,47 @@ func TestDeleteTenantData(t *testing.T) { assert.Equal(t, int64(0), repeatedResp.DeletedFeedbackRecords) assert.Equal(t, int64(0), repeatedResp.DeletedEmbeddings) assert.Equal(t, int64(0), repeatedResp.DeletedWebhooks) + requireNoTenantDataDeleteEvents(t, eventRecorder) +} + +type tenantDataEventRecorder struct { + mu sync.Mutex + events []service.Event +} + +func (r *tenantDataEventRecorder) PublishEvent(_ context.Context, event service.Event) { + r.mu.Lock() + defer r.mu.Unlock() + + r.events = append(r.events, event) +} + +func (r *tenantDataEventRecorder) tenantDataDeleteEventCount() int { + r.mu.Lock() + defer r.mu.Unlock() + + count := 0 + + for _, event := range r.events { + if event.Type == datatypes.FeedbackRecordDeleted || event.Type == datatypes.WebhookDeleted { + count++ + } + } + + return count +} + +func requireNoTenantDataDeleteEvents(t *testing.T, recorder *tenantDataEventRecorder) { + t.Helper() + + deadline := time.Now().Add(150 * time.Millisecond) + for time.Now().Before(deadline) { + if count := recorder.tenantDataDeleteEventCount(); count > 0 { + t.Fatalf("tenant data purge published %d delete events, want 0", count) + } + + time.Sleep(10 * time.Millisecond) + } } func createTenantDataFeedbackRecord( From 218e48381a7ddd49772010942f0661d961d97ff5 Mon Sep 17 00:00:00 2001 From: Tiago Farto Date: Wed, 20 May 2026 13:22:47 +0000 Subject: [PATCH 3/5] chore: Update go to latest version (fix CI problems) --- .github/workflows/api-contract-tests.yml | 2 +- .github/workflows/code-quality.yml | 2 +- .github/workflows/migrations-validate.yml | 2 +- .github/workflows/tests.yml | 6 +++--- .golangci.yml | 2 +- Dockerfile | 2 +- go.mod | 2 +- 7 files changed, 9 insertions(+), 9 deletions(-) diff --git a/.github/workflows/api-contract-tests.yml b/.github/workflows/api-contract-tests.yml index b17390e..9356112 100644 --- a/.github/workflows/api-contract-tests.yml +++ b/.github/workflows/api-contract-tests.yml @@ -53,7 +53,7 @@ jobs: - name: Set up Go uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6.4.0 with: - go-version: '1.26.2' + go-version: '1.26.3' - name: Cache Go modules uses: actions/cache@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5 diff --git a/.github/workflows/code-quality.yml b/.github/workflows/code-quality.yml index 5b14f4a..74b38c2 100644 --- a/.github/workflows/code-quality.yml +++ b/.github/workflows/code-quality.yml @@ -39,7 +39,7 @@ jobs: id: setup-go uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6.4.0 with: - go-version: '1.26.2' + go-version: '1.26.3' - name: Cache Go modules uses: actions/cache@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5 diff --git a/.github/workflows/migrations-validate.yml b/.github/workflows/migrations-validate.yml index d93d8ca..4a9f42e 100644 --- a/.github/workflows/migrations-validate.yml +++ b/.github/workflows/migrations-validate.yml @@ -25,7 +25,7 @@ jobs: - name: Set up Go uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6.4.0 with: - go-version: '1.26.2' + go-version: '1.26.3' - name: Install goose run: go install github.com/pressly/goose/v3/cmd/goose@v3.27.1 diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index da6847f..2202e01 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -31,7 +31,7 @@ jobs: - name: Set up Go uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6.4.0 with: - go-version: '1.26.2' + go-version: '1.26.3' - name: Cache Go modules uses: actions/cache@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5 @@ -85,7 +85,7 @@ jobs: - name: Set up Go uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6.4.0 with: - go-version: '1.26.2' + go-version: '1.26.3' - name: Cache Go modules uses: actions/cache@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5 @@ -148,7 +148,7 @@ jobs: - name: Set up Go uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6.4.0 with: - go-version: '1.26.2' + go-version: '1.26.3' - name: Cache Go modules uses: actions/cache@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5 diff --git a/.golangci.yml b/.golangci.yml index 80bdd4e..864a0cc 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -9,7 +9,7 @@ run: timeout: 5m concurrency: 4 modules-download-mode: readonly - go: "1.26.2" + go: "1.26.3" # Allow linting test files but suppress specific strict rules via exclusion below tests: true diff --git a/Dockerfile b/Dockerfile index 30927c3..7d27af1 100644 --- a/Dockerfile +++ b/Dockerfile @@ -2,7 +2,7 @@ # Stage 1: Build # ============================================================================= # TARGETOS/TARGETARCH are set by Docker Buildx for multi-platform builds (e.g. linux/arm64 on Mac M1). -FROM golang:1.26.2-alpine AS builder +FROM golang:1.26.3-alpine AS builder ARG TARGETOS=linux ARG TARGETARCH diff --git a/go.mod b/go.mod index 51d4410..2d50a99 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/formbricks/hub -go 1.26.2 +go 1.26.3 require ( github.com/go-playground/form/v4 v4.3.0 From d6c4daa560d954d80511f85e7466e609559a7ee5 Mon Sep 17 00:00:00 2001 From: Tiago Farto Date: Fri, 22 May 2026 09:26:44 +0000 Subject: [PATCH 4/5] commit: address pr comments --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 517f59e..acb6832 100644 --- a/Makefile +++ b/Makefile @@ -60,7 +60,7 @@ mcp-smoke: # Run unit tests (fast, no database required) test-unit: @echo "Running unit tests..." - go test ./internal/... -v + go test ./cmd/api ./internal/... -v # Run all tests (unit + integration) test-all: test-unit tests From ffebad674d8022e04599c6c06cb385d343087570 Mon Sep 17 00:00:00 2001 From: Tiago Farto Date: Fri, 22 May 2026 09:40:56 +0000 Subject: [PATCH 5/5] chore: change documentation --- openapi.yaml | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/openapi.yaml b/openapi.yaml index 4db06c0..42bb48c 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -1073,9 +1073,12 @@ paths: - Tenant Data summary: Delete tenant data description: | - Permanently deletes all Hub-owned data for the specified tenant_id. This includes feedback records, - derived embeddings, and webhooks for the tenant. This operation is synchronous and idempotent; - repeated calls return zero counts after the tenant data has already been deleted. + Permanently deletes Hub-owned data for the specified tenant_id. This endpoint is intended for + tenant/account offboarding after the tenant has been deprovisioned and upstream writes for that + tenant have stopped. This includes feedback records, derived embeddings, and webhooks for the + tenant. This operation is synchronous and idempotent; repeated calls return zero counts after the + tenant data has already been deleted. Concurrent writes for the same tenant_id are not serialized + by Hub in this version. No webhook events are published as part of this purge operation. operationId: delete-tenant-data parameters: