Skip to content

Commit 2acbc3e

Browse files
btouchardbtouchard
committed
feat(documents): delete stored file from storage on document deletion
1 parent 0d667cf commit 2acbc3e

3 files changed

Lines changed: 239 additions & 0 deletions

File tree

‎backend/internal/presentation/api/documents/handler.go‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,11 @@ type webhookPublisher interface {
5252
Publish(ctx context.Context, eventType string, payload map[string]interface{}) error
5353
}
5454

55+
// fileDeleter deletes a stored file by its storage key.
56+
type fileDeleter interface {
57+
Delete(ctx context.Context, key string) error
58+
}
59+
5560
// signatureService defines the interface for signature operations
5661
type signatureService interface {
5762
GetDocumentSignatures(ctx context.Context, docID string) ([]*models.Signature, error)
@@ -78,6 +83,7 @@ type Handler struct {
7883
quotaRecorder QuotaRecorder
7984
tenantProvider providers.TenantProvider
8085
auditLogger shared.AuditLogger
86+
storageProvider fileDeleter
8187
baseURL string
8288
}
8389

@@ -116,6 +122,12 @@ func (h *Handler) WithAdminService(adminService adminService, baseURL string) *H
116122
return h
117123
}
118124

125+
// WithStorageProvider sets the storage provider used to delete uploaded files on document deletion.
126+
func (h *Handler) WithStorageProvider(p fileDeleter) *Handler {
127+
h.storageProvider = p
128+
return h
129+
}
130+
119131
// DocumentDTO represents a document data transfer object
120132
type DocumentDTO struct {
121133
ID string `json:"id"`
@@ -1196,6 +1208,16 @@ func (h *Handler) HandleDeleteMyDocument(w http.ResponseWriter, r *http.Request)
11961208
return
11971209
}
11981210

1211+
// Delete the stored file if one exists — best-effort, log on failure
1212+
if h.storageProvider != nil && doc.StorageKey != "" {
1213+
if err := h.storageProvider.Delete(ctx, doc.StorageKey); err != nil {
1214+
logger.Logger.Error("Failed to delete document file from storage",
1215+
"doc_id", doc.DocID,
1216+
"storage_key", doc.StorageKey,
1217+
"error", err.Error())
1218+
}
1219+
}
1220+
11991221
// Decrement document quota counter
12001222
if h.quotaRecorder != nil {
12011223
tenantID := h.getTenantID(ctx)

‎backend/internal/presentation/api/documents/handler_test.go‎

Lines changed: 214 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1153,6 +1153,220 @@ func TestHandler_HandleFindOrCreateDocument_ExistingDoc_NoQuotaRecord(t *testing
11531153
assert.False(t, recorder.recordCalled, "quota record should NOT be called for existing doc")
11541154
}
11551155

1156+
// ============================================================================
1157+
// TESTS - HandleDeleteMyDocument
1158+
// ============================================================================
1159+
1160+
// mockAdminService implements the adminService interface for tests.
1161+
type mockAdminService struct {
1162+
getDocFunc func(ctx context.Context, docID string) (*models.Document, error)
1163+
deleteDocFunc func(ctx context.Context, docID string) error
1164+
}
1165+
1166+
func (m *mockAdminService) GetDocument(ctx context.Context, docID string) (*models.Document, error) {
1167+
if m.getDocFunc != nil {
1168+
return m.getDocFunc(ctx, docID)
1169+
}
1170+
return testDoc, nil
1171+
}
1172+
1173+
func (m *mockAdminService) DeleteDocument(ctx context.Context, docID string) error {
1174+
if m.deleteDocFunc != nil {
1175+
return m.deleteDocFunc(ctx, docID)
1176+
}
1177+
return nil
1178+
}
1179+
1180+
func (m *mockAdminService) UpdateDocumentMetadata(_ context.Context, _ string, _ models.DocumentInput, _ string) (*models.Document, error) {
1181+
return testDoc, nil
1182+
}
1183+
1184+
func (m *mockAdminService) ListExpectedSignersWithStatus(_ context.Context, _ string) ([]*models.ExpectedSignerWithStatus, error) {
1185+
return nil, nil
1186+
}
1187+
1188+
func (m *mockAdminService) GetSignerStats(_ context.Context, _ string) (*models.DocCompletionStats, error) {
1189+
return &models.DocCompletionStats{}, nil
1190+
}
1191+
1192+
func (m *mockAdminService) AddExpectedSigners(_ context.Context, _ string, _ []models.ContactInfo, _ string) error {
1193+
return nil
1194+
}
1195+
1196+
func (m *mockAdminService) RemoveExpectedSigner(_ context.Context, _, _ string) error {
1197+
return nil
1198+
}
1199+
1200+
// mockFileDeleter tracks Delete calls for test assertions.
1201+
type mockFileDeleter struct {
1202+
deletedKeys []string
1203+
deleteErr error
1204+
}
1205+
1206+
func (m *mockFileDeleter) Delete(_ context.Context, key string) error {
1207+
m.deletedKeys = append(m.deletedKeys, key)
1208+
return m.deleteErr
1209+
}
1210+
1211+
func makeDeleteRequest(docID string, user *models.User) *http.Request {
1212+
req := httptest.NewRequest(http.MethodDelete, "/api/v1/users/me/documents/"+docID, nil)
1213+
rctx := chi.NewRouteContext()
1214+
rctx.URLParams.Add("docId", docID)
1215+
ctx := context.WithValue(req.Context(), chi.RouteCtxKey, rctx)
1216+
if user != nil {
1217+
ctx = addUserToContext(ctx, user)
1218+
}
1219+
return req.WithContext(ctx)
1220+
}
1221+
1222+
func TestHandler_HandleDeleteMyDocument_DeletesStoredFile(t *testing.T) {
1223+
t.Parallel()
1224+
1225+
storedDoc := &models.Document{
1226+
DocID: "doc-with-file",
1227+
Title: "Doc With File",
1228+
CreatedBy: testUser.Email,
1229+
StorageKey: "tenants/abc/doc-with-file.pdf",
1230+
StorageProvider: "s3",
1231+
}
1232+
deleter := &mockFileDeleter{}
1233+
1234+
handler := &Handler{
1235+
authorizer: newMockAuthorizer([]string{}, false),
1236+
storageProvider: deleter,
1237+
adminService: &mockAdminService{
1238+
getDocFunc: func(_ context.Context, _ string) (*models.Document, error) {
1239+
return storedDoc, nil
1240+
},
1241+
},
1242+
}
1243+
1244+
rec := httptest.NewRecorder()
1245+
handler.HandleDeleteMyDocument(rec, makeDeleteRequest(storedDoc.DocID, testUser))
1246+
1247+
assert.Equal(t, http.StatusOK, rec.Code)
1248+
require.Len(t, deleter.deletedKeys, 1)
1249+
assert.Equal(t, storedDoc.StorageKey, deleter.deletedKeys[0])
1250+
}
1251+
1252+
func TestHandler_HandleDeleteMyDocument_NoFileNoStorageCall(t *testing.T) {
1253+
t.Parallel()
1254+
1255+
docWithoutFile := &models.Document{
1256+
DocID: "doc-no-file",
1257+
Title: "URL-only doc",
1258+
CreatedBy: testUser.Email,
1259+
URL: "https://example.com/doc.pdf",
1260+
// StorageKey intentionally empty
1261+
}
1262+
deleter := &mockFileDeleter{}
1263+
1264+
handler := &Handler{
1265+
authorizer: newMockAuthorizer([]string{}, false),
1266+
storageProvider: deleter,
1267+
adminService: &mockAdminService{
1268+
getDocFunc: func(_ context.Context, _ string) (*models.Document, error) {
1269+
return docWithoutFile, nil
1270+
},
1271+
},
1272+
}
1273+
1274+
rec := httptest.NewRecorder()
1275+
handler.HandleDeleteMyDocument(rec, makeDeleteRequest(docWithoutFile.DocID, testUser))
1276+
1277+
assert.Equal(t, http.StatusOK, rec.Code)
1278+
assert.Empty(t, deleter.deletedKeys, "Delete must not be called when doc has no storage key")
1279+
}
1280+
1281+
func TestHandler_HandleDeleteMyDocument_NoStorageProvider(t *testing.T) {
1282+
t.Parallel()
1283+
1284+
storedDoc := &models.Document{
1285+
DocID: "doc-with-file",
1286+
Title: "Doc With File",
1287+
CreatedBy: testUser.Email,
1288+
StorageKey: "tenants/abc/doc-with-file.pdf",
1289+
StorageProvider: "s3",
1290+
}
1291+
1292+
// No storage provider set — should succeed without panic
1293+
handler := &Handler{
1294+
authorizer: newMockAuthorizer([]string{}, false),
1295+
adminService: &mockAdminService{
1296+
getDocFunc: func(_ context.Context, _ string) (*models.Document, error) {
1297+
return storedDoc, nil
1298+
},
1299+
},
1300+
}
1301+
1302+
rec := httptest.NewRecorder()
1303+
handler.HandleDeleteMyDocument(rec, makeDeleteRequest(storedDoc.DocID, testUser))
1304+
1305+
assert.Equal(t, http.StatusOK, rec.Code)
1306+
}
1307+
1308+
func TestHandler_HandleDeleteMyDocument_StorageErrorDoesNotFail(t *testing.T) {
1309+
t.Parallel()
1310+
1311+
storedDoc := &models.Document{
1312+
DocID: "doc-with-file",
1313+
Title: "Doc With File",
1314+
CreatedBy: testUser.Email,
1315+
StorageKey: "tenants/abc/doc-with-file.pdf",
1316+
StorageProvider: "s3",
1317+
}
1318+
deleter := &mockFileDeleter{deleteErr: fmt.Errorf("s3 unavailable")}
1319+
1320+
handler := &Handler{
1321+
authorizer: newMockAuthorizer([]string{}, false),
1322+
storageProvider: deleter,
1323+
adminService: &mockAdminService{
1324+
getDocFunc: func(_ context.Context, _ string) (*models.Document, error) {
1325+
return storedDoc, nil
1326+
},
1327+
},
1328+
}
1329+
1330+
rec := httptest.NewRecorder()
1331+
handler.HandleDeleteMyDocument(rec, makeDeleteRequest(storedDoc.DocID, testUser))
1332+
1333+
// Storage error is best-effort: request must still succeed
1334+
assert.Equal(t, http.StatusOK, rec.Code)
1335+
assert.Len(t, deleter.deletedKeys, 1, "Delete must still have been attempted")
1336+
}
1337+
1338+
func TestHandler_HandleDeleteMyDocument_DBErrorSkipsStorageDelete(t *testing.T) {
1339+
t.Parallel()
1340+
1341+
storedDoc := &models.Document{
1342+
DocID: "doc-with-file",
1343+
Title: "Doc With File",
1344+
CreatedBy: testUser.Email,
1345+
StorageKey: "tenants/abc/doc-with-file.pdf",
1346+
StorageProvider: "s3",
1347+
}
1348+
deleter := &mockFileDeleter{}
1349+
1350+
handler := &Handler{
1351+
authorizer: newMockAuthorizer([]string{}, false),
1352+
storageProvider: deleter,
1353+
adminService: &mockAdminService{
1354+
getDocFunc: func(_ context.Context, _ string) (*models.Document, error) {
1355+
return storedDoc, nil
1356+
},
1357+
deleteDocFunc: func(_ context.Context, _ string) error {
1358+
return fmt.Errorf("db error")
1359+
},
1360+
},
1361+
}
1362+
1363+
rec := httptest.NewRecorder()
1364+
handler.HandleDeleteMyDocument(rec, makeDeleteRequest(storedDoc.DocID, testUser))
1365+
1366+
assert.Equal(t, http.StatusInternalServerError, rec.Code)
1367+
assert.Empty(t, deleter.deletedKeys, "Storage Delete must not be called when DB delete fails")
1368+
}
1369+
11561370
func Benchmark_detectReferenceType(b *testing.B) {
11571371
refs := []string{
11581372
"https://example.com/doc.pdf",

‎backend/internal/presentation/api/router.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,9 @@ func NewRouter(cfg RouterConfig) *chi.Mux {
262262
cfg.Authorizer,
263263
).WithAdminService(cfg.AdminService, cfg.BaseURL)
264264

265+
if cfg.StorageProvider != nil {
266+
documentsHandler.WithStorageProvider(cfg.StorageProvider)
267+
}
265268
if cfg.QuotaEnforcer != nil && cfg.TenantProvider != nil {
266269
documentsHandler.WithQuotaRecorder(
267270
&quotaRecorderAdapter{enforcer: cfg.QuotaEnforcer},

0 commit comments

Comments
 (0)