From 5e44b5a6e85dc0fc717400ca34ab968cfce39ee4 Mon Sep 17 00:00:00 2001 From: Johan Briger Date: Sat, 25 Apr 2026 11:17:12 +0200 Subject: [PATCH 01/12] feat: add Flyway migrations and S3 orphaned object cleanup --- pom.xml | 15 ++- .../example/vet1177/Vet1177Application.java | 2 + .../vet1177/entities/OrphanedS3Object.java | 61 +++++++++ .../OrphanedS3ObjectRepository.java | 13 ++ .../vet1177/services/AttachmentService.java | 125 +++++++++--------- .../services/OrphanedS3CleanupWorker.java | 67 ++++++++++ src/main/resources/application-dev.properties | 16 ++- .../db/migration/V1__initial_schema.sql | 107 +++++++++++++++ .../db/migration/V2__init_orphaned_table.sql | 14 ++ .../services/AttachmentServiceTest.java | 7 +- 10 files changed, 351 insertions(+), 76 deletions(-) create mode 100644 src/main/java/org/example/vet1177/entities/OrphanedS3Object.java create mode 100644 src/main/java/org/example/vet1177/repository/OrphanedS3ObjectRepository.java create mode 100644 src/main/java/org/example/vet1177/services/OrphanedS3CleanupWorker.java create mode 100644 src/main/resources/db/migration/V1__initial_schema.sql create mode 100644 src/main/resources/db/migration/V2__init_orphaned_table.sql diff --git a/pom.xml b/pom.xml index 72bc9824..9dc24003 100644 --- a/pom.xml +++ b/pom.xml @@ -81,6 +81,14 @@ org.springframework.boot spring-boot-starter-security + + org.springframework.boot + spring-boot-starter-flyway + + + org.flywaydb + flyway-database-postgresql + org.springframework.boot spring-boot-starter-oauth2-resource-server @@ -103,12 +111,7 @@ - - - src/main/resources - false - - + org.springframework.boot diff --git a/src/main/java/org/example/vet1177/Vet1177Application.java b/src/main/java/org/example/vet1177/Vet1177Application.java index 8a271aeb..fdc032e6 100644 --- a/src/main/java/org/example/vet1177/Vet1177Application.java +++ b/src/main/java/org/example/vet1177/Vet1177Application.java @@ -2,8 +2,10 @@ import org.springframework.boot.SpringApplication; import org.springframework.boot.autoconfigure.SpringBootApplication; +import org.springframework.scheduling.annotation.EnableScheduling; @SpringBootApplication +@EnableScheduling public class Vet1177Application { public static void main(String[] args) { diff --git a/src/main/java/org/example/vet1177/entities/OrphanedS3Object.java b/src/main/java/org/example/vet1177/entities/OrphanedS3Object.java new file mode 100644 index 00000000..12037d28 --- /dev/null +++ b/src/main/java/org/example/vet1177/entities/OrphanedS3Object.java @@ -0,0 +1,61 @@ +package org.example.vet1177.entities; + +import jakarta.persistence.*; +import java.time.Instant; +import java.util.UUID; + +@Entity +@Table(name = "orphaned_s3_objects") +public class OrphanedS3Object { + + @Id + @GeneratedValue(strategy = GenerationType.UUID) + private UUID id; + + @Column(name = "s3_key", unique = true, nullable = false) + private String s3Key; + + @Column(name = "s3_bucket", nullable = false) + private String s3Bucket; + + @Column(name = "created_at", nullable = false, updatable = false) + private Instant createdAt = Instant.now(); + + @Column(name = "retry_count", nullable = false) + private int retryCount = 0; + + @Column(name = "last_attempt_at") + private Instant lastAttemptAt; + + @Column(name = "last_error", columnDefinition = "TEXT") + private String lastError; + + public OrphanedS3Object() {} + + public OrphanedS3Object(String s3Key, String s3Bucket) { + this.s3Key = s3Key; + this.s3Bucket = s3Bucket; + this.createdAt = Instant.now(); + this.retryCount = 0; + } + + public UUID getId() { return id; } + public void setId(UUID id) { this.id = id; } + + public String getS3Key() { return s3Key; } + public void setS3Key(String s3Key) { this.s3Key = s3Key; } + + public String getS3Bucket() { return s3Bucket; } + public void setS3Bucket(String s3Bucket) { this.s3Bucket = s3Bucket; } + + public Instant getCreatedAt() { return createdAt; } + + public int getRetryCount() { return retryCount; } + public void setRetryCount(int retryCount) { this.retryCount = retryCount; } + + public Instant getLastAttemptAt() { return lastAttemptAt; } + public void setLastAttemptAt(Instant lastAttemptAt) { this.lastAttemptAt = lastAttemptAt; } + + public String getLastError() { return lastError; } + public void setLastError(String lastError) { this.lastError = lastError; } +} \ No newline at end of file diff --git a/src/main/java/org/example/vet1177/repository/OrphanedS3ObjectRepository.java b/src/main/java/org/example/vet1177/repository/OrphanedS3ObjectRepository.java new file mode 100644 index 00000000..c8aa4f41 --- /dev/null +++ b/src/main/java/org/example/vet1177/repository/OrphanedS3ObjectRepository.java @@ -0,0 +1,13 @@ +package org.example.vet1177.repository; + +import org.example.vet1177.entities.OrphanedS3Object; +import org.springframework.data.jpa.repository.JpaRepository; +import org.springframework.stereotype.Repository; +import java.util.List; +import java.util.UUID; + +@Repository +public interface OrphanedS3ObjectRepository extends JpaRepository { + + List findTop20ByRetryCountLessThanOrderByLastAttemptAtAsc(int maxRetries); +} diff --git a/src/main/java/org/example/vet1177/services/AttachmentService.java b/src/main/java/org/example/vet1177/services/AttachmentService.java index 9724bde6..e70ad549 100644 --- a/src/main/java/org/example/vet1177/services/AttachmentService.java +++ b/src/main/java/org/example/vet1177/services/AttachmentService.java @@ -5,12 +5,14 @@ import org.example.vet1177.dto.response.attachment.AttachmentResponse; import org.example.vet1177.entities.Attachment; import org.example.vet1177.entities.MedicalRecord; +import org.example.vet1177.entities.OrphanedS3Object; import org.example.vet1177.entities.User; import org.example.vet1177.exception.ResourceNotFoundException; import org.example.vet1177.policy.AttachmentPolicy; import org.example.vet1177.policy.MedicalRecordPolicy; import org.example.vet1177.repository.AttachmentRepository; import org.example.vet1177.repository.MedicalRecordRepository; +import org.example.vet1177.repository.OrphanedS3ObjectRepository; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.stereotype.Service; @@ -18,8 +20,9 @@ import org.springframework.transaction.support.TransactionSynchronization; import org.springframework.transaction.support.TransactionSynchronizationManager; import org.springframework.web.multipart.MultipartFile; -import org.springframework.security.access.AccessDeniedException; + import java.io.IOException; +import java.time.Instant; import java.util.List; import java.util.UUID; @@ -34,24 +37,26 @@ public class AttachmentService { private final AttachmentPolicy attachmentPolicy; private final String bucketName; private final MedicalRecordPolicy medicalRecordPolicy; + private final OrphanedS3ObjectRepository orphanedS3ObjectRepository; public AttachmentService(AttachmentRepository attachmentRepository, FileStorageService fileStorageService, MedicalRecordRepository medicalRecordRepository, AttachmentPolicy attachmentPolicy, AwsS3Properties props, - MedicalRecordPolicy medicalRecordPolicy) { + MedicalRecordPolicy medicalRecordPolicy, + OrphanedS3ObjectRepository orphanedS3ObjectRepository) { this.attachmentRepository = attachmentRepository; this.fileStorageService = fileStorageService; this.medicalRecordRepository = medicalRecordRepository; this.attachmentPolicy = attachmentPolicy; this.bucketName = props.getBucketName(); this.medicalRecordPolicy = medicalRecordPolicy; + this.orphanedS3ObjectRepository = orphanedS3ObjectRepository; } - @Transactional(rollbackFor = Exception.class) - public AttachmentResponse uploadAttachment(User currentUser, MultipartFile file, AttachmentRequest request) { + public AttachmentResponse uploadAttachment(User currentUser, MultipartFile file, AttachmentRequest request) { MedicalRecord record = medicalRecordRepository.findById(request.recordId()) .orElseThrow(() -> new ResourceNotFoundException("MedicalRecord", request.recordId())); @@ -59,19 +64,16 @@ public AttachmentResponse uploadAttachment(User currentUser, MultipartFile file, throw new IllegalArgumentException("Det gÃ¥r inte att ladda upp en tom fil."); } - // Validering attachmentPolicy.canUpload(currentUser, record, file.getContentType(), file.getSize()); String originalName = file.getOriginalFilename(); String sanitizedName = sanitizeFilename(originalName); - // Skapa unik S3-nyckel String s3Key = String.format("records/%s/%s_%s", record.getId(), UUID.randomUUID(), sanitizedName); - // Anropa FileStorageService try { fileStorageService.upload(s3Key, file.getInputStream(), file.getSize(), file.getContentType()); } catch (IOException e) { @@ -82,63 +84,34 @@ public AttachmentResponse uploadAttachment(User currentUser, MultipartFile file, throw new RuntimeException("Kunde inte ladda upp filen till molnlagringen", e); } - // Skapa entitet - try { - Attachment attachment = new Attachment(); - attachment.setMedicalRecord(record); - attachment.setUploadedBy(currentUser); - attachment.setFileName(file.getOriginalFilename()); - attachment.setS3Key(s3Key); - attachment.setS3Bucket(bucketName); - attachment.setFileType(file.getContentType()); - attachment.setFileSizeBytes(file.getSize()); - attachment.setDescription(request.description()); - - attachment = attachmentRepository.saveAndFlush(attachment); - - log.info("Attachment {} successfully persisted for record {}", attachment.getId(), record.getId()); - - return mapToResponse(attachment); - - } catch (Exception e) { - log.error("Database persistence failed for S3 key: {}. Triggering cleanup.", s3Key); - try { - fileStorageService.delete(s3Key); - } catch (Exception deleteEx) { - log.error("CRITICAL: Failed to cleanup S3 object {}!", s3Key, deleteEx); - } - throw new RuntimeException("Kunde inte spara metadata i databasen. Uppladdningen avbröts.", e); - } - } + Attachment attachment = new Attachment(); + attachment.setMedicalRecord(record); + attachment.setUploadedBy(currentUser); + attachment.setFileName(file.getOriginalFilename()); + attachment.setS3Key(s3Key); + attachment.setS3Bucket(bucketName); + attachment.setFileType(file.getContentType()); + attachment.setFileSizeBytes(file.getSize()); + attachment.setDescription(request.description()); - private String sanitizeFilename(String originalFilename) { - if (originalFilename == null || originalFilename.isBlank()) { - return UUID.randomUUID().toString(); - } + attachment = attachmentRepository.saveAndFlush(attachment); - String filename = new java.io.File(originalFilename).getName(); - filename = filename.replaceAll("[^a-zA-Z0-9\\.\\-_]", "_"); + log.info("Attachment {} successfully persisted for record {}", attachment.getId(), record.getId()); + return mapToResponse(attachment); - filename = filename.trim(); - if (filename.isEmpty() || filename.equals(".") || filename.equals("..")) { - return "file_" + System.currentTimeMillis(); + } catch (Exception e) { + log.error("Database persistence failed for S3 key: {}. Triggering cleanup.", s3Key); + try { + fileStorageService.delete(s3Key); + } catch (Exception deleteEx) { + log.error("Upload cleanup failed. Enqueuing {} for background retry.", s3Key); + orphanedS3ObjectRepository.save(new OrphanedS3Object(s3Key, bucketName)); } - return filename; - } - - - @Transactional(readOnly = true) - public AttachmentResponse getAttachment(User currentUser, UUID attachmentId) { - Attachment attachment = attachmentRepository.findById(attachmentId) - .orElseThrow(() -> new ResourceNotFoundException("Attachment", attachmentId)); - - attachmentPolicy.canDownload(currentUser, attachment); - - return mapToResponse(attachment); + throw new RuntimeException("Kunde inte spara metadata i databasen. Uppladdningen avbröts.", e); + } } - @Transactional public void deleteAttachment(User currentUser, UUID attachmentId) { Attachment attachment = attachmentRepository.findById(attachmentId) @@ -147,6 +120,7 @@ public void deleteAttachment(User currentUser, UUID attachmentId) { attachmentPolicy.canDelete(currentUser, attachment); String s3Key = attachment.getS3Key(); + String s3Bucket = attachment.getS3Bucket(); attachmentRepository.delete(attachment); @@ -158,17 +132,33 @@ public void afterCommit() { fileStorageService.delete(s3Key); log.info("S3 object {} deleted after successful DB commit", s3Key); } catch (Exception e) { - log.error("CRITICAL: Failed to delete S3 object {} after DB commit!", s3Key, e); + log.error("S3 deletion failed for {}. Enqueuing for background retry.", s3Key, e); + OrphanedS3Object orphan = new OrphanedS3Object(s3Key, s3Bucket); + orphan.setLastError("Initial deletion failure: " + e.getMessage()); + orphan.setLastAttemptAt(Instant.now()); + orphanedS3ObjectRepository.save(orphan); } } }); } else { - fileStorageService.delete(s3Key); + try { + fileStorageService.delete(s3Key); + } catch (Exception e) { + orphanedS3ObjectRepository.save(new OrphanedS3Object(s3Key, s3Bucket)); + } } - log.info("Attachment {} marked for deletion in database", attachmentId); } + @Transactional(readOnly = true) + public AttachmentResponse getAttachment(User currentUser, UUID attachmentId) { + Attachment attachment = attachmentRepository.findById(attachmentId) + .orElseThrow(() -> new ResourceNotFoundException("Attachment", attachmentId)); + + attachmentPolicy.canDownload(currentUser, attachment); + return mapToResponse(attachment); + } + @Transactional(readOnly = true) public List getAttachmentsByRecord(User currentUser, UUID recordId) { MedicalRecord record = medicalRecordRepository.findById(recordId) @@ -176,15 +166,26 @@ public List getAttachmentsByRecord(User currentUser, UUID re medicalRecordPolicy.canView(currentUser, record); - return attachmentRepository.findByMedicalRecordId(recordId).stream() .map(this::mapToResponse) .toList(); } + private String sanitizeFilename(String originalFilename) { + if (originalFilename == null || originalFilename.isBlank()) { + return UUID.randomUUID().toString(); + } + String filename = new java.io.File(originalFilename).getName(); + filename = filename.replaceAll("[^a-zA-Z0-9\\.\\-_]", "_"); + filename = filename.trim(); + if (filename.isEmpty() || filename.equals(".") || filename.equals("..")) { + return "file_" + System.currentTimeMillis(); + } + return filename; + } + private AttachmentResponse mapToResponse(Attachment attachment) { String downloadUrl = fileStorageService.generatePresignedUrl(attachment.getS3Key()); - return new AttachmentResponse( attachment.getId(), attachment.getMedicalRecord().getId(), diff --git a/src/main/java/org/example/vet1177/services/OrphanedS3CleanupWorker.java b/src/main/java/org/example/vet1177/services/OrphanedS3CleanupWorker.java new file mode 100644 index 00000000..b3385d44 --- /dev/null +++ b/src/main/java/org/example/vet1177/services/OrphanedS3CleanupWorker.java @@ -0,0 +1,67 @@ +package org.example.vet1177.services; + +import org.example.vet1177.entities.OrphanedS3Object; +import org.example.vet1177.repository.OrphanedS3ObjectRepository; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.scheduling.annotation.Scheduled; +import org.springframework.stereotype.Component; +import org.springframework.transaction.annotation.Transactional; + +import java.time.Instant; +import java.util.List; + +@Component +public class OrphanedS3CleanupWorker { + + private static final Logger log = LoggerFactory.getLogger(OrphanedS3CleanupWorker.class); + + private final OrphanedS3ObjectRepository repository; + private final FileStorageService fileStorageService; + + private static final int MAX_RETRIES = 10; + + public OrphanedS3CleanupWorker(OrphanedS3ObjectRepository repository, + FileStorageService fileStorageService) { + this.repository = repository; + this.fileStorageService = fileStorageService; + } + + /** + * Körs var 10:e minut (600 000 millisekunder). + * fixedDelay innebär att nästa körning börjar 10 minuter efter att den förra avslutades. + */ + @Scheduled(fixedDelay = 600000) + @Transactional + public void retryDeletions() { + // Hämta upp till 20 objekt som behöver raderas + List pending = repository.findTop20ByRetryCountLessThanOrderByLastAttemptAtAsc(MAX_RETRIES); + + if (pending.isEmpty()) { + return; + } + + log.info("Starting background cleanup of {} orphaned S3 objects", pending.size()); + + for (OrphanedS3Object orphan : pending) { + try { + // Försök radera frÃ¥n S3/MinIO + fileStorageService.delete(orphan.getS3Key()); + + repository.delete(orphan); + log.info("Successfully cleaned up orphaned S3 object: {}", orphan.getS3Key()); + + } catch (Exception e) { + orphan.setRetryCount(orphan.getRetryCount() + 1); + orphan.setLastAttemptAt(Instant.now()); + + String errorMessage = e.getMessage() != null ? e.getMessage() : "Unknown error during retry"; + orphan.setLastError(errorMessage.substring(0, Math.min(errorMessage.length(), 1024))); + + repository.save(orphan); + log.warn("Retry failed for S3 object {}. Attempt {}/{}", + orphan.getS3Key(), orphan.getRetryCount(), MAX_RETRIES); + } + } + } +} diff --git a/src/main/resources/application-dev.properties b/src/main/resources/application-dev.properties index 0ba6bd55..a0207558 100644 --- a/src/main/resources/application-dev.properties +++ b/src/main/resources/application-dev.properties @@ -3,15 +3,18 @@ # Aktiveras av spring.profiles.active=dev # ============================================ -# Databas ? pekar på lokal Docker-databas +# Databas ? pekar p lokal Docker-databas spring.datasource.url=${DB_URL} spring.datasource.username=${DB_USERNAME} spring.datasource.password=${DB_PASSWORD} -# Visa mer SQL-info i dev +# JPA / Hibernate / Data.sql +spring.jpa.hibernate.ddl-auto=validate spring.jpa.show-sql=true spring.jpa.properties.hibernate.format_sql=true +spring.jpa.defer-datasource-initialization=true +spring.sql.init.mode=always # MinIO ? lokal instans via Docker aws.s3.endpoint=http://localhost:9000 @@ -20,8 +23,7 @@ aws.s3.secret-key=minioadmin aws.s3.bucket-name=vet1177-attachments aws.s3.region=eu-north-1 -# Schema och testdata laddas automatiskt vid uppstart -spring.sql.init.mode=always -spring.sql.init.schema-locations=classpath:schema.sql -spring.sql.init.platform=postgresql -spring.sql.init.data-locations=classpath:data.sql \ No newline at end of file +# Flyway +spring.flyway.enabled=true +spring.flyway.baseline-on-migrate=true +spring.flyway.locations=classpath:db/migration \ No newline at end of file diff --git a/src/main/resources/db/migration/V1__initial_schema.sql b/src/main/resources/db/migration/V1__initial_schema.sql new file mode 100644 index 00000000..e48b7eb8 --- /dev/null +++ b/src/main/resources/db/migration/V1__initial_schema.sql @@ -0,0 +1,107 @@ +CREATE TABLE IF NOT EXISTS clinic ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + name VARCHAR(255) NOT NULL, + address VARCHAR(500) NOT NULL, + phone_number VARCHAR(30), + email VARCHAR(255), + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW() +); + +CREATE TABLE IF NOT EXISTS users ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + name VARCHAR(255) NOT NULL, + email VARCHAR(255) NOT NULL UNIQUE, + password_hash VARCHAR(255) NOT NULL, + role VARCHAR(20) NOT NULL DEFAULT 'OWNER', + clinic_id UUID REFERENCES clinic(id), + is_active BOOLEAN NOT NULL DEFAULT TRUE, + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW() +); + +CREATE TABLE IF NOT EXISTS vet_details ( + user_id UUID PRIMARY KEY REFERENCES users(id), + license_id VARCHAR(50) NOT NULL UNIQUE, + specialization VARCHAR(255), + booking_info VARCHAR(500) +); + +CREATE TABLE IF NOT EXISTS pet ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + owner_id UUID NOT NULL REFERENCES users(id), + clinic_id UUID REFERENCES clinic(id), + name VARCHAR(255) NOT NULL, + species VARCHAR(100) NOT NULL, + breed VARCHAR(255), + date_of_birth DATE, + weight_kg DECIMAL(6,2), + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW() +); + +CREATE TABLE IF NOT EXISTS medical_record ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + title VARCHAR(500) NOT NULL, + description TEXT, + status VARCHAR(20) NOT NULL DEFAULT 'OPEN', + pet_id UUID NOT NULL REFERENCES pet(id), + owner_id UUID NOT NULL REFERENCES users(id), + clinic_id UUID NOT NULL REFERENCES clinic(id), + assigned_vet_id UUID REFERENCES users(id), + created_by UUID NOT NULL REFERENCES users(id), + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + updated_by UUID REFERENCES users(id), + updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + closed_at TIMESTAMPTZ +); + +CREATE TABLE IF NOT EXISTS comment ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + record_id UUID NOT NULL REFERENCES medical_record(id), + author_id UUID NOT NULL REFERENCES users(id), + body TEXT NOT NULL, + comment_type VARCHAR(32) NOT NULL DEFAULT 'OWNER_MESSAGE', + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW() +); + +ALTER TABLE comment ADD COLUMN IF NOT EXISTS comment_type VARCHAR(32) NOT NULL DEFAULT 'OWNER_MESSAGE'; + +CREATE TABLE IF NOT EXISTS attachment ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + record_id UUID NOT NULL REFERENCES medical_record(id) ON DELETE CASCADE, + uploaded_by UUID REFERENCES users(id) ON DELETE SET NULL, + file_name VARCHAR(500) NOT NULL, + description VARCHAR(500), + s3_key VARCHAR(1000) NOT NULL UNIQUE, + s3_bucket VARCHAR(255) NOT NULL, + file_type VARCHAR(100), + file_size_bytes BIGINT, + uploaded_at TIMESTAMPTZ NOT NULL DEFAULT NOW() +); + +-- Migration för befintliga databaser som skapades innan description-kolumnen lades till. +-- CREATE TABLE IF NOT EXISTS ovan är no-op om tabellen redan finns, sÃ¥ kolumnen mÃ¥ste +-- läggas till explicit. Samma mönster som comment_type ovan. +ALTER TABLE attachment ADD COLUMN IF NOT EXISTS description VARCHAR(500); + +CREATE TABLE IF NOT EXISTS activity_log ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + record_id UUID REFERENCES medical_record(id), + action VARCHAR(50) NOT NULL, + entity_type VARCHAR(50) NOT NULL, + entity_id UUID, + performed_by UUID NOT NULL REFERENCES users(id), + details TEXT, + performed_at TIMESTAMPTZ NOT NULL DEFAULT NOW() +); + +-- Index +CREATE INDEX IF NOT EXISTS idx_users_email ON users(email); +CREATE INDEX IF NOT EXISTS idx_users_clinic ON users(clinic_id); +CREATE INDEX IF NOT EXISTS idx_record_pet ON medical_record(pet_id); +CREATE INDEX IF NOT EXISTS idx_record_status ON medical_record(status); +CREATE INDEX IF NOT EXISTS idx_comment_record ON comment(record_id, created_at); +CREATE INDEX IF NOT EXISTS idx_attachment_record ON attachment(record_id); +CREATE INDEX IF NOT EXISTS idx_log_record ON activity_log(record_id, performed_at); \ No newline at end of file diff --git a/src/main/resources/db/migration/V2__init_orphaned_table.sql b/src/main/resources/db/migration/V2__init_orphaned_table.sql new file mode 100644 index 00000000..9d77f11e --- /dev/null +++ b/src/main/resources/db/migration/V2__init_orphaned_table.sql @@ -0,0 +1,14 @@ +-- Skapar tabellen för misslyckade raderingar +CREATE TABLE orphaned_s3_objects ( + id UUID PRIMARY KEY, + s3_key VARCHAR(512) NOT NULL, + s3_bucket VARCHAR(255) NOT NULL, + created_at TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT CURRENT_TIMESTAMP, + retry_count INT NOT NULL DEFAULT 0, + last_attempt_at TIMESTAMP WITH TIME ZONE, + last_error TEXT, + CONSTRAINT uk_orphaned_s3_key UNIQUE (s3_key) +); + + +CREATE INDEX idx_orphaned_s3_retry ON orphaned_s3_objects (retry_count, last_attempt_at); \ No newline at end of file diff --git a/src/test/java/org/example/vet1177/services/AttachmentServiceTest.java b/src/test/java/org/example/vet1177/services/AttachmentServiceTest.java index 80e21945..6fdd49ec 100644 --- a/src/test/java/org/example/vet1177/services/AttachmentServiceTest.java +++ b/src/test/java/org/example/vet1177/services/AttachmentServiceTest.java @@ -9,6 +9,7 @@ import org.example.vet1177.policy.MedicalRecordPolicy; import org.example.vet1177.repository.AttachmentRepository; import org.example.vet1177.repository.MedicalRecordRepository; +import org.example.vet1177.repository.OrphanedS3ObjectRepository; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -47,6 +48,9 @@ class AttachmentServiceTest { @Mock private MedicalRecordPolicy medicalRecordPolicy; + @Mock + private OrphanedS3ObjectRepository orphanedS3ObjectRepository; + private AttachmentService attachmentService; private User vetUser; @@ -69,7 +73,8 @@ void setUp() { medicalRecordRepository, attachmentPolicy, props, - medicalRecordPolicy + medicalRecordPolicy, + orphanedS3ObjectRepository ); vetUser = new User("Dr. Sara Lindqvist", "sara@vet.se", "hash", Role.VET); From 67e9f4b5b67a95ab09f79892aeb24fecfe0ad832 Mon Sep 17 00:00:00 2001 From: Johan Briger Date: Sat, 25 Apr 2026 12:04:44 +0200 Subject: [PATCH 02/12] test: disable Flyway in integration tests --- src/test/resources/application-test.properties | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/test/resources/application-test.properties b/src/test/resources/application-test.properties index 0fcfdc88..becc8639 100644 --- a/src/test/resources/application-test.properties +++ b/src/test/resources/application-test.properties @@ -36,4 +36,6 @@ jwt.expiration-ms=86400000 # Mindre output i tester spring.jpa.show-sql=false -spring.jpa.properties.hibernate.format_sql=false \ No newline at end of file +spring.jpa.properties.hibernate.format_sql=false + +spring.flyway.enabled=false \ No newline at end of file From 77a0c64881b5aa6b247ba36b3d2fc9c823cd2661 Mon Sep 17 00:00:00 2001 From: Johan Briger Date: Sat, 25 Apr 2026 12:56:23 +0200 Subject: [PATCH 03/12] fix: prioritize new orphans in cleanup queue --- .../repository/OrphanedS3ObjectRepository.java | 11 ++++++++++- .../vet1177/services/OrphanedS3CleanupWorker.java | 6 +++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/example/vet1177/repository/OrphanedS3ObjectRepository.java b/src/main/java/org/example/vet1177/repository/OrphanedS3ObjectRepository.java index c8aa4f41..7239b29a 100644 --- a/src/main/java/org/example/vet1177/repository/OrphanedS3ObjectRepository.java +++ b/src/main/java/org/example/vet1177/repository/OrphanedS3ObjectRepository.java @@ -1,7 +1,10 @@ package org.example.vet1177.repository; import org.example.vet1177.entities.OrphanedS3Object; +import org.springframework.data.domain.Pageable; import org.springframework.data.jpa.repository.JpaRepository; +import org.springframework.data.jpa.repository.Query; +import org.springframework.data.repository.query.Param; import org.springframework.stereotype.Repository; import java.util.List; import java.util.UUID; @@ -9,5 +12,11 @@ @Repository public interface OrphanedS3ObjectRepository extends JpaRepository { - List findTop20ByRetryCountLessThanOrderByLastAttemptAtAsc(int maxRetries); + @Query(""" + SELECT o FROM OrphanedS3Object o + WHERE o.retryCount < :maxRetries + ORDER BY o.lastAttemptAt ASC NULLS FIRST, o.createdAt ASC + """) + + List findNextBatchToProcess(@Param("maxRetries") int maxRetries, Pageable pageable); } diff --git a/src/main/java/org/example/vet1177/services/OrphanedS3CleanupWorker.java b/src/main/java/org/example/vet1177/services/OrphanedS3CleanupWorker.java index b3385d44..e7a6836e 100644 --- a/src/main/java/org/example/vet1177/services/OrphanedS3CleanupWorker.java +++ b/src/main/java/org/example/vet1177/services/OrphanedS3CleanupWorker.java @@ -4,6 +4,7 @@ import org.example.vet1177.repository.OrphanedS3ObjectRepository; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.data.domain.PageRequest; import org.springframework.scheduling.annotation.Scheduled; import org.springframework.stereotype.Component; import org.springframework.transaction.annotation.Transactional; @@ -35,7 +36,10 @@ public OrphanedS3CleanupWorker(OrphanedS3ObjectRepository repository, @Transactional public void retryDeletions() { // Hämta upp till 20 objekt som behöver raderas - List pending = repository.findTop20ByRetryCountLessThanOrderByLastAttemptAtAsc(MAX_RETRIES); + List pending = repository.findNextBatchToProcess( + MAX_RETRIES, + PageRequest.of(0, 20) + ); if (pending.isEmpty()) { return; From 2844da8c10f05c3dfa743dc303abdd21ba400693 Mon Sep 17 00:00:00 2001 From: Johan Briger Date: Sat, 25 Apr 2026 13:36:05 +0200 Subject: [PATCH 04/12] - Added OrphanedS3Enqueuer with REQUIRES_NEW to ensure cleanup records persist even if main transactions roll back. - Updated AttachmentService to use the new enqueuer and refined post-commit deletion logic using TransactionSynchronization. - Optimized cleanup queue with 'NULLS FIRST' ordering to prevent retries from starving new deletion tasks. - Refactored AttachmentServiceTest to reflect new service architecture and verified error-handling edge cases. --- .../vet1177/services/AttachmentService.java | 21 ++++------ .../vet1177/services/OrphanedS3Enqueuer.java | 41 +++++++++++++++++++ .../services/AttachmentServiceTest.java | 27 ++++++++++-- 3 files changed, 73 insertions(+), 16 deletions(-) create mode 100644 src/main/java/org/example/vet1177/services/OrphanedS3Enqueuer.java diff --git a/src/main/java/org/example/vet1177/services/AttachmentService.java b/src/main/java/org/example/vet1177/services/AttachmentService.java index e70ad549..55ce0360 100644 --- a/src/main/java/org/example/vet1177/services/AttachmentService.java +++ b/src/main/java/org/example/vet1177/services/AttachmentService.java @@ -5,14 +5,12 @@ import org.example.vet1177.dto.response.attachment.AttachmentResponse; import org.example.vet1177.entities.Attachment; import org.example.vet1177.entities.MedicalRecord; -import org.example.vet1177.entities.OrphanedS3Object; import org.example.vet1177.entities.User; import org.example.vet1177.exception.ResourceNotFoundException; import org.example.vet1177.policy.AttachmentPolicy; import org.example.vet1177.policy.MedicalRecordPolicy; import org.example.vet1177.repository.AttachmentRepository; import org.example.vet1177.repository.MedicalRecordRepository; -import org.example.vet1177.repository.OrphanedS3ObjectRepository; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.stereotype.Service; @@ -22,7 +20,6 @@ import org.springframework.web.multipart.MultipartFile; import java.io.IOException; -import java.time.Instant; import java.util.List; import java.util.UUID; @@ -37,7 +34,7 @@ public class AttachmentService { private final AttachmentPolicy attachmentPolicy; private final String bucketName; private final MedicalRecordPolicy medicalRecordPolicy; - private final OrphanedS3ObjectRepository orphanedS3ObjectRepository; + private final OrphanedS3Enqueuer orphanedS3Enqueuer; public AttachmentService(AttachmentRepository attachmentRepository, FileStorageService fileStorageService, @@ -45,14 +42,14 @@ public AttachmentService(AttachmentRepository attachmentRepository, AttachmentPolicy attachmentPolicy, AwsS3Properties props, MedicalRecordPolicy medicalRecordPolicy, - OrphanedS3ObjectRepository orphanedS3ObjectRepository) { + OrphanedS3Enqueuer orphanedS3Enqueuer) { this.attachmentRepository = attachmentRepository; this.fileStorageService = fileStorageService; this.medicalRecordRepository = medicalRecordRepository; this.attachmentPolicy = attachmentPolicy; this.bucketName = props.getBucketName(); this.medicalRecordPolicy = medicalRecordPolicy; - this.orphanedS3ObjectRepository = orphanedS3ObjectRepository; + this.orphanedS3Enqueuer = orphanedS3Enqueuer; } @Transactional(rollbackFor = Exception.class) @@ -106,7 +103,7 @@ public AttachmentResponse uploadAttachment(User currentUser, MultipartFile file, fileStorageService.delete(s3Key); } catch (Exception deleteEx) { log.error("Upload cleanup failed. Enqueuing {} for background retry.", s3Key); - orphanedS3ObjectRepository.save(new OrphanedS3Object(s3Key, bucketName)); + orphanedS3Enqueuer.enqueue(s3Key, bucketName, "Upload transaction failed"); } throw new RuntimeException("Kunde inte spara metadata i databasen. Uppladdningen avbröts.", e); } @@ -133,10 +130,8 @@ public void afterCommit() { log.info("S3 object {} deleted after successful DB commit", s3Key); } catch (Exception e) { log.error("S3 deletion failed for {}. Enqueuing for background retry.", s3Key, e); - OrphanedS3Object orphan = new OrphanedS3Object(s3Key, s3Bucket); - orphan.setLastError("Initial deletion failure: " + e.getMessage()); - orphan.setLastAttemptAt(Instant.now()); - orphanedS3ObjectRepository.save(orphan); + + orphanedS3Enqueuer.enqueue(s3Key, s3Bucket, "S3 deletion failed after commit: " + e.getMessage()); } } }); @@ -144,7 +139,7 @@ public void afterCommit() { try { fileStorageService.delete(s3Key); } catch (Exception e) { - orphanedS3ObjectRepository.save(new OrphanedS3Object(s3Key, s3Bucket)); + orphanedS3Enqueuer.enqueue(s3Key, bucketName, "Delete failed: " + e.getMessage()); } } log.info("Attachment {} marked for deletion in database", attachmentId); @@ -176,7 +171,7 @@ private String sanitizeFilename(String originalFilename) { return UUID.randomUUID().toString(); } String filename = new java.io.File(originalFilename).getName(); - filename = filename.replaceAll("[^a-zA-Z0-9\\.\\-_]", "_"); + filename = filename.replaceAll("[^a-zA-Z0-9.\\-_]", "_"); filename = filename.trim(); if (filename.isEmpty() || filename.equals(".") || filename.equals("..")) { return "file_" + System.currentTimeMillis(); diff --git a/src/main/java/org/example/vet1177/services/OrphanedS3Enqueuer.java b/src/main/java/org/example/vet1177/services/OrphanedS3Enqueuer.java new file mode 100644 index 00000000..699b4138 --- /dev/null +++ b/src/main/java/org/example/vet1177/services/OrphanedS3Enqueuer.java @@ -0,0 +1,41 @@ +package org.example.vet1177.services; + +import org.example.vet1177.entities.OrphanedS3Object; +import org.example.vet1177.repository.OrphanedS3ObjectRepository; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Propagation; +import org.springframework.transaction.annotation.Transactional; + +import java.time.Instant; + +@Service +public class OrphanedS3Enqueuer { + + private final OrphanedS3ObjectRepository repository; + + public OrphanedS3Enqueuer(OrphanedS3ObjectRepository repository) { + this.repository = repository; + } + + /** + * REQUIRES_NEW tvingar Spring att pausa den nuvarande transaktionen + * och starta en helt ny, oberoende transaktion för just detta sparning. + */ + @Transactional(propagation = Propagation.REQUIRES_NEW) + public void enqueue(String s3Key, String bucket, String reason) { + OrphanedS3Object orphan = new OrphanedS3Object(s3Key, bucket); + + // Vi sätter lastAttemptAt till nu direkt sÃ¥ att kön vet att den är ny + orphan.setLastAttemptAt(null); // Eller Instant.now() beroende pÃ¥ hur vi vill att NULLS FIRST ska reagera + + if (reason != null) { + // Säkerställ att vi inte kraschar pÃ¥ för lÃ¥nga felmeddelanden + String truncatedReason = reason.length() > 1024 + ? reason.substring(0, 1021) + "..." + : reason; + orphan.setLastError(truncatedReason); + } + + repository.save(orphan); + } +} diff --git a/src/test/java/org/example/vet1177/services/AttachmentServiceTest.java b/src/test/java/org/example/vet1177/services/AttachmentServiceTest.java index 6fdd49ec..b5fda69c 100644 --- a/src/test/java/org/example/vet1177/services/AttachmentServiceTest.java +++ b/src/test/java/org/example/vet1177/services/AttachmentServiceTest.java @@ -9,7 +9,6 @@ import org.example.vet1177.policy.MedicalRecordPolicy; import org.example.vet1177.repository.AttachmentRepository; import org.example.vet1177.repository.MedicalRecordRepository; -import org.example.vet1177.repository.OrphanedS3ObjectRepository; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -49,7 +48,7 @@ class AttachmentServiceTest { private MedicalRecordPolicy medicalRecordPolicy; @Mock - private OrphanedS3ObjectRepository orphanedS3ObjectRepository; + private OrphanedS3Enqueuer orphanedS3Enqueuer; private AttachmentService attachmentService; @@ -74,7 +73,8 @@ void setUp() { attachmentPolicy, props, medicalRecordPolicy, - orphanedS3ObjectRepository + orphanedS3Enqueuer + ); vetUser = new User("Dr. Sara Lindqvist", "sara@vet.se", "hash", Role.VET); @@ -300,4 +300,25 @@ void delete_whenNotFound_shouldThrowResourceNotFoundException() { verify(attachmentRepository, never()).delete(any()); } + + @Test + void upload_whenDbFailsAndS3DeleteFails_shouldEnqueueOrphan() { + MockMultipartFile file = new MockMultipartFile( + "file", "bild.jpg", "image/jpeg", new byte[]{1, 2, 3}); + + when(medicalRecordRepository.findById(recordId)).thenReturn(Optional.of(record)); + + // 1. Tvinga DB att krascha + when(attachmentRepository.saveAndFlush(any())).thenThrow(new RuntimeException("DB Error")); + + // 2. Tvinga även raderingen att krascha (det är dÃ¥ den hamnar i kön!) + doThrow(new RuntimeException("S3 Delete Error")) + .when(fileStorageService).delete(anyString()); + + assertThatThrownBy(() -> attachmentService.uploadAttachment(vetUser, file, new AttachmentRequest(recordId, "En bild"))) + .isInstanceOf(RuntimeException.class); + + // Nu kommer kön att anropas! + verify(orphanedS3Enqueuer).enqueue(anyString(), eq("test-bucket"), anyString()); + } } From 86f24b10598505d7d2176a8f8d2a0cda234803aa Mon Sep 17 00:00:00 2001 From: Johan Briger Date: Sat, 25 Apr 2026 18:32:09 +0200 Subject: [PATCH 05/12] refactor: simplify delete logic by removing unreachable else branch --- .../example/vet1177/services/AttachmentService.java | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/example/vet1177/services/AttachmentService.java b/src/main/java/org/example/vet1177/services/AttachmentService.java index 55ce0360..25e0143a 100644 --- a/src/main/java/org/example/vet1177/services/AttachmentService.java +++ b/src/main/java/org/example/vet1177/services/AttachmentService.java @@ -120,8 +120,7 @@ public void deleteAttachment(User currentUser, UUID attachmentId) { String s3Bucket = attachment.getS3Bucket(); attachmentRepository.delete(attachment); - - if (TransactionSynchronizationManager.isSynchronizationActive()) { + TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() { @Override public void afterCommit() { @@ -135,13 +134,7 @@ public void afterCommit() { } } }); - } else { - try { - fileStorageService.delete(s3Key); - } catch (Exception e) { - orphanedS3Enqueuer.enqueue(s3Key, bucketName, "Delete failed: " + e.getMessage()); - } - } + log.info("Attachment {} marked for deletion in database", attachmentId); } From b690d2b63e05edc4c3122524fc58401307d945a3 Mon Sep 17 00:00:00 2001 From: Johan Briger Date: Sat, 25 Apr 2026 21:58:25 +0200 Subject: [PATCH 06/12] refactor(attachment): enforce transaction-aware S3 deletion - Remove unsafe fallback for S3 deletion outside of active transactions. - Refactor deleteAttachment to rely solely on TransactionSynchronization. - Implement fail-fast behavior by requiring an active transaction context. - Update AttachmentServiceTest to verify post-commit synchronization using Mockito static mocks and ArgumentCaptors. --- .../vet1177/services/AttachmentService.java | 2 +- .../services/OrphanedS3CleanupWorker.java | 33 ++---------- .../vet1177/services/OrphanedS3Processor.java | 52 +++++++++++++++++++ .../services/AttachmentServiceTest.java | 29 +++++++++-- 4 files changed, 83 insertions(+), 33 deletions(-) create mode 100644 src/main/java/org/example/vet1177/services/OrphanedS3Processor.java diff --git a/src/main/java/org/example/vet1177/services/AttachmentService.java b/src/main/java/org/example/vet1177/services/AttachmentService.java index 25e0143a..b9b906e1 100644 --- a/src/main/java/org/example/vet1177/services/AttachmentService.java +++ b/src/main/java/org/example/vet1177/services/AttachmentService.java @@ -120,7 +120,7 @@ public void deleteAttachment(User currentUser, UUID attachmentId) { String s3Bucket = attachment.getS3Bucket(); attachmentRepository.delete(attachment); - + TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() { @Override public void afterCommit() { diff --git a/src/main/java/org/example/vet1177/services/OrphanedS3CleanupWorker.java b/src/main/java/org/example/vet1177/services/OrphanedS3CleanupWorker.java index e7a6836e..6cafb58d 100644 --- a/src/main/java/org/example/vet1177/services/OrphanedS3CleanupWorker.java +++ b/src/main/java/org/example/vet1177/services/OrphanedS3CleanupWorker.java @@ -7,9 +7,7 @@ import org.springframework.data.domain.PageRequest; import org.springframework.scheduling.annotation.Scheduled; import org.springframework.stereotype.Component; -import org.springframework.transaction.annotation.Transactional; -import java.time.Instant; import java.util.List; @Component @@ -18,24 +16,18 @@ public class OrphanedS3CleanupWorker { private static final Logger log = LoggerFactory.getLogger(OrphanedS3CleanupWorker.class); private final OrphanedS3ObjectRepository repository; - private final FileStorageService fileStorageService; + private final OrphanedS3Processor processor; // Ny injektion private static final int MAX_RETRIES = 10; public OrphanedS3CleanupWorker(OrphanedS3ObjectRepository repository, - FileStorageService fileStorageService) { + OrphanedS3Processor processor) { this.repository = repository; - this.fileStorageService = fileStorageService; + this.processor = processor; } - /** - * Körs var 10:e minut (600 000 millisekunder). - * fixedDelay innebär att nästa körning börjar 10 minuter efter att den förra avslutades. - */ @Scheduled(fixedDelay = 600000) - @Transactional public void retryDeletions() { - // Hämta upp till 20 objekt som behöver raderas List pending = repository.findNextBatchToProcess( MAX_RETRIES, PageRequest.of(0, 20) @@ -48,24 +40,7 @@ public void retryDeletions() { log.info("Starting background cleanup of {} orphaned S3 objects", pending.size()); for (OrphanedS3Object orphan : pending) { - try { - // Försök radera frÃ¥n S3/MinIO - fileStorageService.delete(orphan.getS3Key()); - - repository.delete(orphan); - log.info("Successfully cleaned up orphaned S3 object: {}", orphan.getS3Key()); - - } catch (Exception e) { - orphan.setRetryCount(orphan.getRetryCount() + 1); - orphan.setLastAttemptAt(Instant.now()); - - String errorMessage = e.getMessage() != null ? e.getMessage() : "Unknown error during retry"; - orphan.setLastError(errorMessage.substring(0, Math.min(errorMessage.length(), 1024))); - - repository.save(orphan); - log.warn("Retry failed for S3 object {}. Attempt {}/{}", - orphan.getS3Key(), orphan.getRetryCount(), MAX_RETRIES); - } + processor.processOrphan(orphan, MAX_RETRIES); } } } diff --git a/src/main/java/org/example/vet1177/services/OrphanedS3Processor.java b/src/main/java/org/example/vet1177/services/OrphanedS3Processor.java new file mode 100644 index 00000000..37a6eb46 --- /dev/null +++ b/src/main/java/org/example/vet1177/services/OrphanedS3Processor.java @@ -0,0 +1,52 @@ +package org.example.vet1177.services; + +import org.example.vet1177.entities.OrphanedS3Object; +import org.example.vet1177.repository.OrphanedS3ObjectRepository; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.stereotype.Component; +import org.springframework.transaction.annotation.Propagation; +import org.springframework.transaction.annotation.Transactional; + +import java.time.Instant; + +@Component +public class OrphanedS3Processor { + + private static final Logger log = LoggerFactory.getLogger(OrphanedS3Processor.class); + private final OrphanedS3ObjectRepository repository; + private final FileStorageService fileStorageService; + + public OrphanedS3Processor(OrphanedS3ObjectRepository repository, + FileStorageService fileStorageService) { + this.repository = repository; + this.fileStorageService = fileStorageService; + } + + @Transactional(propagation = Propagation.REQUIRES_NEW) + public void processOrphan(OrphanedS3Object orphan, int maxRetries) { + try { + // Försök radera frÃ¥n S3/MinIO + fileStorageService.delete(orphan.getS3Key()); + + // Ta bort frÃ¥n kön vid framgÃ¥ng + repository.delete(orphan); + log.info("Successfully cleaned up orphaned S3 object: {}", orphan.getS3Key()); + + } catch (Exception e) { + // Vid fel: uppdatera räknare och felmeddelande i en egen transaktion + orphan.setRetryCount(orphan.getRetryCount() + 1); + orphan.setLastAttemptAt(Instant.now()); + + String errorMessage = e.getMessage() != null ? e.getMessage() : "Unknown error during retry"; + if (errorMessage.length() > 1024) { + errorMessage = errorMessage.substring(0, 1021) + "..."; + } + orphan.setLastError(errorMessage); + + repository.save(orphan); + log.warn("Retry failed for S3 object {}. Attempt {}/{}", + orphan.getS3Key(), orphan.getRetryCount(), maxRetries); + } + } +} \ No newline at end of file diff --git a/src/test/java/org/example/vet1177/services/AttachmentServiceTest.java b/src/test/java/org/example/vet1177/services/AttachmentServiceTest.java index b5fda69c..05024bc9 100644 --- a/src/test/java/org/example/vet1177/services/AttachmentServiceTest.java +++ b/src/test/java/org/example/vet1177/services/AttachmentServiceTest.java @@ -12,10 +12,13 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.mock.web.MockMultipartFile; import org.springframework.test.util.ReflectionTestUtils; +import org.springframework.transaction.support.TransactionSynchronization; +import org.springframework.transaction.support.TransactionSynchronizationManager; import java.util.List; import java.util.Optional; @@ -266,29 +269,49 @@ void getByRecord_whenRecordNotFound_shouldThrowResourceNotFoundException() { @Test void delete_shouldDeleteFromRepository() { + try (var mockedStatic = mockStatic(TransactionSynchronizationManager.class)) { + mockedStatic.when(TransactionSynchronizationManager::isSynchronizationActive).thenReturn(true); when(attachmentRepository.findById(attachmentId)).thenReturn(Optional.of(attachment)); attachmentService.deleteAttachment(vetUser, attachmentId); verify(attachmentRepository).delete(attachment); + mockedStatic.verify(() -> TransactionSynchronizationManager.registerSynchronization(any())); + } } @Test void delete_shouldCallPolicyCanDelete() { + try (var mockedStatic = mockStatic(TransactionSynchronizationManager.class)) { + mockedStatic.when(TransactionSynchronizationManager::isSynchronizationActive).thenReturn(true); when(attachmentRepository.findById(attachmentId)).thenReturn(Optional.of(attachment)); attachmentService.deleteAttachment(vetUser, attachmentId); verify(attachmentPolicy).canDelete(vetUser, attachment); + mockedStatic.verify(() -> TransactionSynchronizationManager.registerSynchronization(any())); + } + } @Test void delete_shouldCallFileStorageDelete() { - when(attachmentRepository.findById(attachmentId)).thenReturn(Optional.of(attachment)); + try (var mockedStatic = mockStatic(TransactionSynchronizationManager.class)) { + // Berätta för Spring att transaktionen är aktiv + mockedStatic.when(TransactionSynchronizationManager::isSynchronizationActive).thenReturn(true); - attachmentService.deleteAttachment(vetUser, attachmentId); + when(attachmentRepository.findById(attachmentId)).thenReturn(Optional.of(attachment)); + + ArgumentCaptor syncCaptor = ArgumentCaptor.forClass(TransactionSynchronization.class); + + attachmentService.deleteAttachment(vetUser, attachmentId); + + mockedStatic.verify(() -> TransactionSynchronizationManager.registerSynchronization(syncCaptor.capture())); + + syncCaptor.getValue().afterCommit(); - verify(fileStorageService).delete(attachment.getS3Key()); + verify(fileStorageService).delete(attachment.getS3Key()); + } } @Test From d07b23062f2f284a71c1b8c766c333f512968a73 Mon Sep 17 00:00:00 2001 From: Johan Briger Date: Sat, 25 Apr 2026 22:14:15 +0200 Subject: [PATCH 07/12] fix(cleanup): improve error logging by preserving stack traces --- .../java/org/example/vet1177/services/OrphanedS3Enqueuer.java | 4 ++-- .../org/example/vet1177/services/OrphanedS3Processor.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/example/vet1177/services/OrphanedS3Enqueuer.java b/src/main/java/org/example/vet1177/services/OrphanedS3Enqueuer.java index 699b4138..56ad73b1 100644 --- a/src/main/java/org/example/vet1177/services/OrphanedS3Enqueuer.java +++ b/src/main/java/org/example/vet1177/services/OrphanedS3Enqueuer.java @@ -25,8 +25,8 @@ public OrphanedS3Enqueuer(OrphanedS3ObjectRepository repository) { public void enqueue(String s3Key, String bucket, String reason) { OrphanedS3Object orphan = new OrphanedS3Object(s3Key, bucket); - // Vi sätter lastAttemptAt till nu direkt sÃ¥ att kön vet att den är ny - orphan.setLastAttemptAt(null); // Eller Instant.now() beroende pÃ¥ hur vi vill att NULLS FIRST ska reagera + + orphan.setLastAttemptAt(null); if (reason != null) { // Säkerställ att vi inte kraschar pÃ¥ för lÃ¥nga felmeddelanden diff --git a/src/main/java/org/example/vet1177/services/OrphanedS3Processor.java b/src/main/java/org/example/vet1177/services/OrphanedS3Processor.java index 37a6eb46..97043a61 100644 --- a/src/main/java/org/example/vet1177/services/OrphanedS3Processor.java +++ b/src/main/java/org/example/vet1177/services/OrphanedS3Processor.java @@ -46,7 +46,7 @@ public void processOrphan(OrphanedS3Object orphan, int maxRetries) { repository.save(orphan); log.warn("Retry failed for S3 object {}. Attempt {}/{}", - orphan.getS3Key(), orphan.getRetryCount(), maxRetries); + orphan.getS3Key(), orphan.getRetryCount(), maxRetries, e); } } } \ No newline at end of file From 72c334c20a9a02f1e2e3b459b20982efb516a089 Mon Sep 17 00:00:00 2001 From: Johan Briger Date: Sat, 25 Apr 2026 22:48:02 +0200 Subject: [PATCH 08/12] refactor(db): migrate demo data to Flyway and fix schema race condition - Move demo data from data.sql to V3__insert_demo_data.sql. - Disable spring.sql.init.mode to let Flyway handle all data seeding. - Set spring.jpa.hibernate.ddl-auto=none to prevent validation errors during Flyway initialization (missing activity_log table). - Clean up V3 script to include explicit timestamps and default values. --- src/main/resources/application-dev.properties | 4 +-- .../db/migration/V3__insert_demo_data.sql | 32 +++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) create mode 100644 src/main/resources/db/migration/V3__insert_demo_data.sql diff --git a/src/main/resources/application-dev.properties b/src/main/resources/application-dev.properties index a0207558..c518cf43 100644 --- a/src/main/resources/application-dev.properties +++ b/src/main/resources/application-dev.properties @@ -10,11 +10,11 @@ spring.datasource.password=${DB_PASSWORD} # JPA / Hibernate / Data.sql -spring.jpa.hibernate.ddl-auto=validate +spring.jpa.hibernate.ddl-auto=none spring.jpa.show-sql=true spring.jpa.properties.hibernate.format_sql=true spring.jpa.defer-datasource-initialization=true -spring.sql.init.mode=always +spring.sql.init.mode=never # MinIO ? lokal instans via Docker aws.s3.endpoint=http://localhost:9000 diff --git a/src/main/resources/db/migration/V3__insert_demo_data.sql b/src/main/resources/db/migration/V3__insert_demo_data.sql new file mode 100644 index 00000000..6d733d5a --- /dev/null +++ b/src/main/resources/db/migration/V3__insert_demo_data.sql @@ -0,0 +1,32 @@ +-- V3__insert_demo_data.sql + +-- Klinik +INSERT INTO clinic (id, name, address, phone_number, email, created_at, updated_at) +VALUES ('a1b2c3d4-e5f6-4a5b-8c9d-e0f1a2b3c4d5', 'Djurkliniken Centrum', 'Storgatan 1, Stockholm', '08-123456', 'centrum@klinik.se', NOW(), NOW()); + +-- Users +INSERT INTO users (id, name, email, password_hash, role, clinic_id, is_active, created_at, updated_at) +VALUES + ('c3d4e5f6-a7b8-4c5d-0e1f-a2b3c4d5e6f7', 'Anna Svensson', 'anna@test.se', '$2a$10$mxL7BIFi9S2jdA867vmuDeRe4SZ/krIiHsCzvOdh/3j5nviJ0G2xy', 'OWNER', NULL, true, NOW(), NOW()), + ('b1c2d3e4-f5a6-4b7c-8d9e-f0a1b2c3d4e5', 'Lars Johansson', 'lars@test.se', '$2a$10$mxL7BIFi9S2jdA867vmuDeRe4SZ/krIiHsCzvOdh/3j5nviJ0G2xy', 'OWNER', NULL, true, NOW(), NOW()), + ('d4e5f6a7-b8c9-4d5e-1f2a-b3c4d5e6f7a8', 'Erik Veterinär', 'erik@klinik.se', '$2a$10$mxL7BIFi9S2jdA867vmuDeRe4SZ/krIiHsCzvOdh/3j5nviJ0G2xy', 'VET', 'a1b2c3d4-e5f6-4a5b-8c9d-e0f1a2b3c4d5', true, NOW(), NOW()), + ('e5f6a7b8-c9d0-4e5f-2a3b-c4d5e6f7a8b9', 'Sara Admin', 'sara@admin.se', '$2a$10$mxL7BIFi9S2jdA867vmuDeRe4SZ/krIiHsCzvOdh/3j5nviJ0G2xy', 'ADMIN', NULL, true, NOW(), NOW()); + +-- Pets +INSERT INTO pet (id, owner_id, clinic_id, name, species, breed, weight_kg, created_at, updated_at) +VALUES + ('f6a7b8c9-d0e1-4f5a-3b4c-d5e6f7a8b9c0', 'c3d4e5f6-a7b8-4c5d-0e1f-a2b3c4d5e6f7', 'a1b2c3d4-e5f6-4a5b-8c9d-e0f1a2b3c4d5', 'Fido', 'Hund', 'Labrador', 28.5, NOW(), NOW()), + ('a7b8c9d0-e1f2-4a5b-4c5d-e6f7a8b9c0d1', 'c3d4e5f6-a7b8-4c5d-0e1f-a2b3c4d5e6f7', 'a1b2c3d4-e5f6-4a5b-8c9d-e0f1a2b3c4d5', 'Missan', 'Katt', 'Persisk', 4.2, NOW(), NOW()), + ('c8d9e0f1-a2b3-4c4d-5e6f-a7b8c9d0e1f2', 'b1c2d3e4-f5a6-4b7c-8d9e-f0a1b2c3d4e5', 'a1b2c3d4-e5f6-4a5b-8c9d-e0f1a2b3c4d5', 'Birk', 'Hund', 'Golden Retriever', 32.0, NOW(), NOW()), + ('d9e0f1a2-b3c4-4d5e-6f7a-b8c9d0e1f2a3', 'b1c2d3e4-f5a6-4b7c-8d9e-f0a1b2c3d4e5', 'a1b2c3d4-e5f6-4a5b-8c9d-e0f1a2b3c4d5', 'Misse', 'Katt', 'Maine Coon', 6.8, NOW(), NOW()); + +-- Medical Records +INSERT INTO medical_record (id, title, description, status, pet_id, owner_id, clinic_id, created_by, created_at, updated_at) +VALUES + ('b8c9d0e1-f2a3-4b5c-5d6e-f7a8b9c0d1e2', 'Fido haltar', 'Hunden har haltat sedan igÃ¥r.', 'OPEN', 'f6a7b8c9-d0e1-4f5a-3b4c-d5e6f7a8b9c0', 'c3d4e5f6-a7b8-4c5d-0e1f-a2b3c4d5e6f7', 'a1b2c3d4-e5f6-4a5b-8c9d-e0f1a2b3c4d5', 'c3d4e5f6-a7b8-4c5d-0e1f-a2b3c4d5e6f7', NOW(), NOW()), + ('c9d0e1f2-a3b4-4c5d-6e7f-a8b9c0d1e2f3', 'Missan äter inte', 'Katten har inte ätit pÃ¥ 2 dagar.', 'IN_PROGRESS', 'a7b8c9d0-e1f2-4a5b-4c5d-e6f7a8b9c0d1', 'c3d4e5f6-a7b8-4c5d-0e1f-a2b3c4d5e6f7', 'a1b2c3d4-e5f6-4a5b-8c9d-e0f1a2b3c4d5', 'd4e5f6a7-b8c9-4d5e-1f2a-b3c4d5e6f7a8', NOW(), NOW()); + +-- Comments +INSERT INTO comment (id, record_id, author_id, body, comment_type, created_at, updated_at) +VALUES + ('a2b3c4d5-e6f7-4a5b-9c0d-e1f2a3b4c5d6', 'c9d0e1f2-a3b4-4c5d-6e7f-a8b9c0d1e2f3', 'c3d4e5f6-a7b8-4c5d-0e1f-a2b3c4d5e6f7', 'Hon dricker vatten men vägrar all mat.', 'OWNER_MESSAGE', NOW(), NOW()); \ No newline at end of file From a448cc53f7cb1941fcd0105e85d1401ec8c538d6 Mon Sep 17 00:00:00 2001 From: Johan Briger Date: Sat, 25 Apr 2026 23:04:47 +0200 Subject: [PATCH 09/12] security(db): isolate demo data to dev-only flyway location --- src/main/resources/application-dev.properties | 2 +- .../resources/db/migration/{ => dev}/V3__insert_demo_data.sql | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename src/main/resources/db/migration/{ => dev}/V3__insert_demo_data.sql (100%) diff --git a/src/main/resources/application-dev.properties b/src/main/resources/application-dev.properties index c518cf43..084e016e 100644 --- a/src/main/resources/application-dev.properties +++ b/src/main/resources/application-dev.properties @@ -26,4 +26,4 @@ aws.s3.region=eu-north-1 # Flyway spring.flyway.enabled=true spring.flyway.baseline-on-migrate=true -spring.flyway.locations=classpath:db/migration \ No newline at end of file +spring.flyway.locations=classpath:db/migration,classpath:db/migration/dev \ No newline at end of file diff --git a/src/main/resources/db/migration/V3__insert_demo_data.sql b/src/main/resources/db/migration/dev/V3__insert_demo_data.sql similarity index 100% rename from src/main/resources/db/migration/V3__insert_demo_data.sql rename to src/main/resources/db/migration/dev/V3__insert_demo_data.sql From 438b754d8fa83aa3bb6b356ff0b97358e1f97c98 Mon Sep 17 00:00:00 2001 From: Johan Briger Date: Sat, 25 Apr 2026 23:13:12 +0200 Subject: [PATCH 10/12] fix(service): improve error handling and traceability in AttachmentService --- .../vet1177/services/AttachmentService.java | 35 +++++++++++-------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/src/main/java/org/example/vet1177/services/AttachmentService.java b/src/main/java/org/example/vet1177/services/AttachmentService.java index b9b906e1..2ccff9a3 100644 --- a/src/main/java/org/example/vet1177/services/AttachmentService.java +++ b/src/main/java/org/example/vet1177/services/AttachmentService.java @@ -98,12 +98,18 @@ public AttachmentResponse uploadAttachment(User currentUser, MultipartFile file, return mapToResponse(attachment); } catch (Exception e) { - log.error("Database persistence failed for S3 key: {}. Triggering cleanup.", s3Key); + // FIX: Inkluderar stacktrace (e) och bevarar felkedjan + log.error("Database persistence failed for S3 key: {}. Triggering cleanup.", s3Key, e); try { fileStorageService.delete(s3Key); } catch (Exception deleteEx) { - log.error("Upload cleanup failed. Enqueuing {} for background retry.", s3Key); - orphanedS3Enqueuer.enqueue(s3Key, bucketName, "Upload transaction failed"); + // FIX: Loggar cleanup-felet separat och skapar en informativ anledning för kön + log.error("Upload cleanup failed for {}. Enqueuing for background retry.", s3Key, deleteEx); + + String errorReason = String.format("DB Error: %s. Cleanup Error: %s", + e.getClass().getSimpleName(), deleteEx.getMessage()); + + orphanedS3Enqueuer.enqueue(s3Key, bucketName, errorReason); } throw new RuntimeException("Kunde inte spara metadata i databasen. Uppladdningen avbröts.", e); } @@ -121,19 +127,18 @@ public void deleteAttachment(User currentUser, UUID attachmentId) { attachmentRepository.delete(attachment); - TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() { - @Override - public void afterCommit() { - try { - fileStorageService.delete(s3Key); - log.info("S3 object {} deleted after successful DB commit", s3Key); - } catch (Exception e) { - log.error("S3 deletion failed for {}. Enqueuing for background retry.", s3Key, e); - - orphanedS3Enqueuer.enqueue(s3Key, s3Bucket, "S3 deletion failed after commit: " + e.getMessage()); - } + TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() { + @Override + public void afterCommit() { + try { + fileStorageService.delete(s3Key); + log.info("S3 object {} deleted after successful DB commit", s3Key); + } catch (Exception e) { + log.error("S3 deletion failed for {}. Enqueuing for background retry.", s3Key, e); + orphanedS3Enqueuer.enqueue(s3Key, s3Bucket, "S3 deletion failed after commit: " + e.getMessage()); } - }); + } + }); log.info("Attachment {} marked for deletion in database", attachmentId); } From e16586cef188c2c63b7031a13746eb70383be3f4 Mon Sep 17 00:00:00 2001 From: Johan Briger Date: Sat, 25 Apr 2026 23:19:25 +0200 Subject: [PATCH 11/12] fix(cleanup): add alerting for orphaned S3 objects exceeding max retries --- .../OrphanedS3ObjectRepository.java | 2 ++ .../services/OrphanedS3CleanupWorker.java | 25 ++++++++++++------- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/example/vet1177/repository/OrphanedS3ObjectRepository.java b/src/main/java/org/example/vet1177/repository/OrphanedS3ObjectRepository.java index 7239b29a..6ca666f9 100644 --- a/src/main/java/org/example/vet1177/repository/OrphanedS3ObjectRepository.java +++ b/src/main/java/org/example/vet1177/repository/OrphanedS3ObjectRepository.java @@ -19,4 +19,6 @@ public interface OrphanedS3ObjectRepository extends JpaRepository findNextBatchToProcess(@Param("maxRetries") int maxRetries, Pageable pageable); + + long countByRetryCountGreaterThanEqual(int maxRetries); } diff --git a/src/main/java/org/example/vet1177/services/OrphanedS3CleanupWorker.java b/src/main/java/org/example/vet1177/services/OrphanedS3CleanupWorker.java index 6cafb58d..31e1929f 100644 --- a/src/main/java/org/example/vet1177/services/OrphanedS3CleanupWorker.java +++ b/src/main/java/org/example/vet1177/services/OrphanedS3CleanupWorker.java @@ -16,7 +16,7 @@ public class OrphanedS3CleanupWorker { private static final Logger log = LoggerFactory.getLogger(OrphanedS3CleanupWorker.class); private final OrphanedS3ObjectRepository repository; - private final OrphanedS3Processor processor; // Ny injektion + private final OrphanedS3Processor processor; private static final int MAX_RETRIES = 10; @@ -26,21 +26,28 @@ public OrphanedS3CleanupWorker(OrphanedS3ObjectRepository repository, this.processor = processor; } - @Scheduled(fixedDelay = 600000) + @Scheduled(fixedDelay = 600000) // 10 minuter public void retryDeletions() { List pending = repository.findNextBatchToProcess( MAX_RETRIES, PageRequest.of(0, 20) ); - if (pending.isEmpty()) { - return; + // Processera batchen om den inte är tom + if (!pending.isEmpty()) { + log.info("Starting background cleanup of {} orphaned S3 objects", pending.size()); + for (OrphanedS3Object orphan : pending) { + processor.processOrphan(orphan, MAX_RETRIES); + } } - log.info("Starting background cleanup of {} orphaned S3 objects", pending.size()); - - for (OrphanedS3Object orphan : pending) { - processor.processOrphan(orphan, MAX_RETRIES); + // ALERT-LOGIK: Kontrollera om det finns objekt som har misslyckats permanent + // (retry_count >= MAX_RETRIES). Dessa dyker inte upp i 'pending' ovan. + long permanentFailures = repository.countByRetryCountGreaterThanEqual(MAX_RETRIES); + if (permanentFailures > 0) { + log.error("ALERT: {} orphaned S3 objects have exceeded MAX_RETRIES={}. " + + "These objects will no longer be retried and require manual cleanup in S3.", + permanentFailures, MAX_RETRIES); } } -} +} \ No newline at end of file From 56102f98c53fd7105732b3cb264073f07a883ca5 Mon Sep 17 00:00:00 2001 From: Johan Briger Date: Sat, 25 Apr 2026 23:26:01 +0200 Subject: [PATCH 12/12] fix(cleanup): make OrphanedS3Enqueuer idempotent using find-or-create --- .../OrphanedS3ObjectRepository.java | 3 ++ .../vet1177/services/OrphanedS3Enqueuer.java | 37 ++++++++++++++----- 2 files changed, 30 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/example/vet1177/repository/OrphanedS3ObjectRepository.java b/src/main/java/org/example/vet1177/repository/OrphanedS3ObjectRepository.java index 6ca666f9..4e0236a4 100644 --- a/src/main/java/org/example/vet1177/repository/OrphanedS3ObjectRepository.java +++ b/src/main/java/org/example/vet1177/repository/OrphanedS3ObjectRepository.java @@ -7,6 +7,7 @@ import org.springframework.data.repository.query.Param; import org.springframework.stereotype.Repository; import java.util.List; +import java.util.Optional; import java.util.UUID; @Repository @@ -21,4 +22,6 @@ public interface OrphanedS3ObjectRepository extends JpaRepository findNextBatchToProcess(@Param("maxRetries") int maxRetries, Pageable pageable); long countByRetryCountGreaterThanEqual(int maxRetries); + + Optional findByS3Key(String s3Key); } diff --git a/src/main/java/org/example/vet1177/services/OrphanedS3Enqueuer.java b/src/main/java/org/example/vet1177/services/OrphanedS3Enqueuer.java index 56ad73b1..1f92ab1d 100644 --- a/src/main/java/org/example/vet1177/services/OrphanedS3Enqueuer.java +++ b/src/main/java/org/example/vet1177/services/OrphanedS3Enqueuer.java @@ -2,6 +2,8 @@ import org.example.vet1177.entities.OrphanedS3Object; import org.example.vet1177.repository.OrphanedS3ObjectRepository; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Propagation; import org.springframework.transaction.annotation.Transactional; @@ -11,6 +13,7 @@ @Service public class OrphanedS3Enqueuer { + private static final Logger log = LoggerFactory.getLogger(OrphanedS3Enqueuer.class); private final OrphanedS3ObjectRepository repository; public OrphanedS3Enqueuer(OrphanedS3ObjectRepository repository) { @@ -19,23 +22,37 @@ public OrphanedS3Enqueuer(OrphanedS3ObjectRepository repository) { /** * REQUIRES_NEW tvingar Spring att pausa den nuvarande transaktionen - * och starta en helt ny, oberoende transaktion för just detta sparning. + * och starta en helt ny, oberoende transaktion för just denna sparning. + * Metoden är nu idempotent (find-or-create). */ @Transactional(propagation = Propagation.REQUIRES_NEW) public void enqueue(String s3Key, String bucket, String reason) { - OrphanedS3Object orphan = new OrphanedS3Object(s3Key, bucket); - - - orphan.setLastAttemptAt(null); + // Förbered det trunkerade felmeddelandet + String truncatedReason = null; if (reason != null) { - // Säkerställ att vi inte kraschar pÃ¥ för lÃ¥nga felmeddelanden - String truncatedReason = reason.length() > 1024 + truncatedReason = reason.length() > 1024 ? reason.substring(0, 1021) + "..." : reason; - orphan.setLastError(truncatedReason); } - repository.save(orphan); + final String finalReason = truncatedReason; + + // "Upsert"-logik: Hitta befintlig eller skapa ny + repository.findByS3Key(s3Key).ifPresentOrElse( + existing -> { + log.debug("Orphaned S3 key already exists: {}. Updating error info.", s3Key); + existing.setLastError(finalReason); + existing.setLastAttemptAt(null); // Ã…terställ sÃ¥ att Worker kan plocka upp den direkt + repository.save(existing); + }, + () -> { + log.info("Enqueuing new orphaned S3 object: {}", s3Key); + OrphanedS3Object newOrphan = new OrphanedS3Object(s3Key, bucket); + newOrphan.setLastError(finalReason); + newOrphan.setLastAttemptAt(null); + repository.save(newOrphan); + } + ); } -} +} \ No newline at end of file