From 36fc43d04c87b6df7ec904cdbbd625fbbab47989 Mon Sep 17 00:00:00 2001 From: Linus Westling Date: Wed, 22 Apr 2026 16:58:03 +0200 Subject: [PATCH 1/7] S3 hardening --- pom.xml | 4 + .../ProjektArendehanteringApplication.java | 2 + .../application/service/DocumentService.java | 100 +++++++--------- .../service/FailedS3DeletionService.java | 107 +++++++++++++++++ .../application/service/S3RetryExecutor.java | 108 ++++++++++++++++++ .../persistence/FailedS3DeletionEntity.java | 42 +++++++ .../FailedS3DeletionRepository.java | 13 +++ src/main/resources/application.properties | 10 +- .../service/DocumentServiceTest.java | 66 ++++++++++- .../service/FailedS3DeletionServiceTest.java | 104 +++++++++++++++++ .../service/S3RetryExecutorTest.java | 53 +++++++++ src/test/resources/application.properties | 8 +- 12 files changed, 554 insertions(+), 63 deletions(-) create mode 100644 src/main/java/org/example/projektarendehantering/application/service/FailedS3DeletionService.java create mode 100644 src/main/java/org/example/projektarendehantering/application/service/S3RetryExecutor.java create mode 100644 src/main/java/org/example/projektarendehantering/infrastructure/persistence/FailedS3DeletionEntity.java create mode 100644 src/main/java/org/example/projektarendehantering/infrastructure/persistence/FailedS3DeletionRepository.java create mode 100644 src/test/java/org/example/projektarendehantering/application/service/FailedS3DeletionServiceTest.java create mode 100644 src/test/java/org/example/projektarendehantering/application/service/S3RetryExecutorTest.java diff --git a/pom.xml b/pom.xml index 1e07dc9..be513b0 100644 --- a/pom.xml +++ b/pom.xml @@ -130,6 +130,10 @@ io.awspring.cloud spring-cloud-aws-starter-s3 + + org.springframework.retry + spring-retry + diff --git a/src/main/java/org/example/projektarendehantering/ProjektArendehanteringApplication.java b/src/main/java/org/example/projektarendehantering/ProjektArendehanteringApplication.java index d9a4a6c..ba719fb 100644 --- a/src/main/java/org/example/projektarendehantering/ProjektArendehanteringApplication.java +++ b/src/main/java/org/example/projektarendehantering/ProjektArendehanteringApplication.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 ProjektArendehanteringApplication { public static void main(String[] args) { diff --git a/src/main/java/org/example/projektarendehantering/application/service/DocumentService.java b/src/main/java/org/example/projektarendehantering/application/service/DocumentService.java index f08e89f..001ccf2 100644 --- a/src/main/java/org/example/projektarendehantering/application/service/DocumentService.java +++ b/src/main/java/org/example/projektarendehantering/application/service/DocumentService.java @@ -11,8 +11,6 @@ import org.springframework.beans.factory.annotation.Value; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; -import org.springframework.transaction.support.TransactionSynchronization; -import org.springframework.transaction.support.TransactionSynchronizationManager; import org.springframework.web.multipart.MultipartFile; import java.io.IOException; @@ -29,6 +27,8 @@ public class DocumentService { private final DocumentRepository documentRepository; private final CaseRepository caseRepository; private final S3Template s3Template; + private final S3RetryExecutor s3RetryExecutor; + private final FailedS3DeletionService failedS3DeletionService; private final DocumentMapper documentMapper; private final AuditService auditService; @@ -60,31 +60,17 @@ public DocumentDTO uploadDocument(Actor actor, UUID caseId, MultipartFile file) String s3Key = UUID.randomUUID().toString() + "-" + originalFilename; - try { - ObjectMetadata metadata = ObjectMetadata.builder() - .contentType(file.getContentType()) - .build(); - s3Template.upload(bucket, s3Key, file.getInputStream(), metadata); - } catch (Exception e) { - log.error("S3 upload failed for bucket: {}, key: {}. Error: {}", bucket, s3Key, e.getMessage(), e); - throw new AppException("S3_UPLOAD_FAILED", "Failed to upload file to S3"); - } - - boolean syncActive = TransactionSynchronizationManager.isSynchronizationActive(); - if (syncActive) { - TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() { - @Override - public void afterCompletion(int status) { - if (status != TransactionSynchronization.STATUS_COMMITTED) { - try { - s3Template.deleteObject(bucket, s3Key); - } catch (Exception cleanupEx) { - log.error("Failed to cleanup rolled-back S3 object {}", s3Key, cleanupEx); - } - } - } - }); - } + ObjectMetadata metadata = ObjectMetadata.builder() + .contentType(file.getContentType()) + .build(); + s3RetryExecutor.execute("upload", context -> { + try { + s3Template.upload(bucket, s3Key, file.getInputStream(), metadata); + } catch (IOException ioException) { + throw new AppException("S3_UPLOAD_STREAM_FAILED", "Failed to read upload input stream", ioException); + } + return null; + }); DocumentEntity entity = DocumentEntity.builder() .fileName(originalFilename) @@ -127,12 +113,16 @@ public void afterCompletion(int status) { } return documentMapper.toDTO(saved); } catch (RuntimeException ex) { - if (!syncActive) { - try { + try { + s3RetryExecutor.execute("delete", context -> { s3Template.deleteObject(bucket, s3Key); - } catch (Exception cleanupEx) { - log.error("Failed to cleanup orphaned S3 object {}", s3Key, cleanupEx); - } + return null; + }); + } catch (Exception cleanupEx) { + log.error("Failed to cleanup orphaned S3 object {}", s3Key, cleanupEx); + failedS3DeletionService.enqueue(bucket, s3Key, cleanupEx); + recordS3Audit(actor, caseEntity.getId(), "DOCUMENT_UPLOAD_COMPENSATION_QUEUED", + "Queued failed upload compensation cleanup for retry", s3Key); } throw ex; } @@ -159,7 +149,7 @@ public S3Resource downloadDocument(Actor actor, UUID documentId) { validateAccess(actor, entity.getCaseEntity()); - return s3Template.download(bucket, entity.getS3Key()); + return s3RetryExecutor.execute("download", context -> s3Template.download(bucket, entity.getS3Key())); } @Transactional @@ -169,37 +159,31 @@ public void deleteDocument(Actor actor, UUID documentId) { validateAccess(actor, entity.getCaseEntity()); - // Save S3 key before deletion String s3Key = entity.getS3Key(); - - // 1. Delete DB entity inside the transaction documentRepository.delete(entity); - - // 2. Delete S3 object *after* successful commit (if in a transaction) - if (TransactionSynchronizationManager.isSynchronizationActive()) { - TransactionSynchronizationManager.registerSynchronization( - new TransactionSynchronization() { - @Override - public void afterCommit() { - try { - s3Template.deleteObject(bucket, s3Key); - } catch (Exception e) { - // DB is already committed — log but do not throw - log.error("Failed to delete S3 object {} after DB commit", s3Key, e); - } - } - } - ); - } else { - // No active transaction (e.g. in unit test) - try { + try { + s3RetryExecutor.execute("delete", context -> { s3Template.deleteObject(bucket, s3Key); - } catch (Exception e) { - log.error("Failed to delete S3 object {} (no active transaction)", s3Key, e); - } + return null; + }); + } catch (Exception e) { + log.error("Failed to delete S3 object {} after DB delete", s3Key, e); + failedS3DeletionService.enqueue(bucket, s3Key, e); } } + private void recordS3Audit(Actor actor, UUID caseId, String eventName, String description, String s3Key) { + auditService.record(AuditEventEntity.builder() + .caseId(caseId) + .eventName(eventName) + .description(description) + .actorId(actor != null ? actor.userId() : null) + .actorRole(actor != null && actor.role() != null ? actor.role().name() : null) + .queryString("bucket=" + bucket + "&s3Key=" + s3Key) + .occurredAt(Instant.now()) + .build()); + } + public DocumentEntity getEntity(Actor actor, UUID documentId) { DocumentEntity entity = documentRepository.findById(documentId) diff --git a/src/main/java/org/example/projektarendehantering/application/service/FailedS3DeletionService.java b/src/main/java/org/example/projektarendehantering/application/service/FailedS3DeletionService.java new file mode 100644 index 0000000..141ddc2 --- /dev/null +++ b/src/main/java/org/example/projektarendehantering/application/service/FailedS3DeletionService.java @@ -0,0 +1,107 @@ +package org.example.projektarendehantering.application.service; + +import io.awspring.cloud.s3.S3Template; +import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; +import org.example.projektarendehantering.infrastructure.persistence.FailedS3DeletionEntity; +import org.example.projektarendehantering.infrastructure.persistence.FailedS3DeletionRepository; +import org.example.projektarendehantering.infrastructure.persistence.AuditEventEntity; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.data.domain.PageRequest; +import org.springframework.scheduling.annotation.Scheduled; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; + +import java.time.Instant; +import java.time.temporal.ChronoUnit; +import java.util.List; + +@Slf4j +@Service +@RequiredArgsConstructor +public class FailedS3DeletionService { + + private final FailedS3DeletionRepository failedS3DeletionRepository; + private final S3Template s3Template; + private final S3RetryExecutor s3RetryExecutor; + private final AuditService auditService; + + @Value("${app.s3.failed-delete.batch-size:20}") + private int batchSize; + + @Value("${app.s3.failed-delete.retry-delay-seconds:60}") + private long retryDelaySeconds; + + @Transactional + public void enqueue(String bucket, String s3Key, Exception e) { + if (failedS3DeletionRepository.existsByBucketAndS3Key(bucket, s3Key)) { + log.warn("Failed S3 deletion already queued. bucket={}, key={}", bucket, s3Key); + return; + } + Instant now = Instant.now(); + FailedS3DeletionEntity entity = FailedS3DeletionEntity.builder() + .bucket(bucket) + .s3Key(s3Key) + .attemptCount(0) + .nextAttemptAt(now.plus(retryDelaySeconds, ChronoUnit.SECONDS)) + .createdAt(now) + .updatedAt(now) + .lastError(trimError(e)) + .build(); + failedS3DeletionRepository.save(entity); + log.warn("Queued failed S3 deletion for retry. bucket={}, key={}", bucket, s3Key); + auditService.record(AuditEventEntity.builder() + .eventName("DOCUMENT_S3_DELETE_QUEUED") + .description("Queued failed S3 deletion for retry") + .queryString("bucket=" + bucket + "&s3Key=" + s3Key + "&error=" + trimError(e)) + .occurredAt(now) + .build()); + } + + @Scheduled(fixedDelayString = "${app.s3.failed-delete.scheduler-delay-ms:30000}") + @Transactional + public void processPendingDeletions() { + List items = failedS3DeletionRepository + .findByNextAttemptAtBeforeOrderByCreatedAtAsc(Instant.now(), PageRequest.of(0, batchSize)); + + for (FailedS3DeletionEntity item : items) { + try { + s3RetryExecutor.execute("delete", context -> { + s3Template.deleteObject(item.getBucket(), item.getS3Key()); + return null; + }); + failedS3DeletionRepository.delete(item); + log.info("Recovered failed S3 deletion. bucket={}, key={}", item.getBucket(), item.getS3Key()); + auditService.record(AuditEventEntity.builder() + .eventName("DOCUMENT_S3_DELETE_RECOVERED") + .description("Recovered failed S3 deletion from retry queue") + .queryString("bucket=" + item.getBucket() + "&s3Key=" + item.getS3Key() + "&attempts=" + item.getAttemptCount()) + .occurredAt(Instant.now()) + .build()); + } catch (Exception ex) { + item.setAttemptCount(item.getAttemptCount() + 1); + item.setUpdatedAt(Instant.now()); + item.setNextAttemptAt(Instant.now().plus(retryDelaySeconds, ChronoUnit.SECONDS)); + item.setLastError(trimError(ex)); + failedS3DeletionRepository.save(item); + log.warn("Failed retrying S3 deletion. bucket={}, key={}, attempts={}", + item.getBucket(), item.getS3Key(), item.getAttemptCount(), ex); + auditService.record(AuditEventEntity.builder() + .eventName("DOCUMENT_S3_DELETE_RETRY_FAILED") + .description("Retry failed for queued S3 deletion") + .queryString("bucket=" + item.getBucket() + "&s3Key=" + item.getS3Key() + + "&attempts=" + item.getAttemptCount() + "&error=" + trimError(ex)) + .occurredAt(Instant.now()) + .build()); + } + } + } + + private String trimError(Exception e) { + String message = e.getMessage(); + if (message == null) { + return e.getClass().getSimpleName(); + } + return message.length() > 900 ? message.substring(0, 900) : message; + } +} diff --git a/src/main/java/org/example/projektarendehantering/application/service/S3RetryExecutor.java b/src/main/java/org/example/projektarendehantering/application/service/S3RetryExecutor.java new file mode 100644 index 0000000..0f3f143 --- /dev/null +++ b/src/main/java/org/example/projektarendehantering/application/service/S3RetryExecutor.java @@ -0,0 +1,108 @@ +package org.example.projektarendehantering.application.service; + +import org.example.projektarendehantering.common.AppException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.retry.RetryCallback; +import org.springframework.retry.RetryContext; +import org.springframework.retry.backoff.ExponentialBackOffPolicy; +import org.springframework.retry.policy.SimpleRetryPolicy; +import org.springframework.retry.support.RetryTemplate; +import org.springframework.stereotype.Component; +import software.amazon.awssdk.awscore.exception.AwsServiceException; +import software.amazon.awssdk.core.exception.SdkClientException; +import software.amazon.awssdk.services.s3.model.S3Exception; + +import java.util.Set; + +@Component +public class S3RetryExecutor { + + private static final Logger log = LoggerFactory.getLogger(S3RetryExecutor.class); + private static final Set RETRYABLE_HTTP_STATUS = Set.of(408, 429, 500, 502, 503, 504); + + private final RetryTemplate retryTemplate; + + public S3RetryExecutor( + @Value("${app.s3.retry.max-attempts:3}") int maxAttempts, + @Value("${app.s3.retry.initial-backoff-ms:200}") long initialBackoffMs, + @Value("${app.s3.retry.max-backoff-ms:2000}") long maxBackoffMs + ) { + this.retryTemplate = createRetryTemplate(maxAttempts, initialBackoffMs, maxBackoffMs); + } + + public T execute(String operationName, RetryCallback callback) { + try { + return retryTemplate.execute(callback); + } catch (RuntimeException ex) { + throw mapToAppException(operationName, ex); + } + } + + public boolean isRetryable(Throwable throwable) { + Throwable root = rootCause(throwable); + + if (root instanceof S3Exception s3Exception) { + return RETRYABLE_HTTP_STATUS.contains(s3Exception.statusCode()) || isRetryableAwsErrorCode(s3Exception.awsErrorDetails() != null + ? s3Exception.awsErrorDetails().errorCode() + : null); + } + if (root instanceof AwsServiceException awsServiceException) { + return RETRYABLE_HTTP_STATUS.contains(awsServiceException.statusCode()); + } + return root instanceof SdkClientException; + } + + private AppException mapToAppException(String operationName, RuntimeException ex) { + String code; + String message; + if (isRetryable(ex)) { + code = "S3_SERVICE_DEGRADED"; + message = "Temporary S3 issue while trying to " + operationName; + } else { + code = "S3_" + operationName.toUpperCase() + "_FAILED"; + message = "Failed to " + operationName + " file in S3"; + } + log.error("S3 {} failed. code={}, retryable={}, error={}", operationName, code, isRetryable(ex), ex.getMessage(), ex); + return new AppException(code, message, ex); + } + + private RetryTemplate createRetryTemplate(int maxAttempts, long initialBackoffMs, long maxBackoffMs) { + RetryTemplate template = new RetryTemplate(); + template.setRetryPolicy(new SimpleRetryPolicy(maxAttempts) { + @Override + public boolean canRetry(RetryContext context) { + if (!super.canRetry(context)) { + return false; + } + Throwable lastThrowable = context.getLastThrowable(); + return lastThrowable == null || isRetryable(lastThrowable); + } + }); + ExponentialBackOffPolicy backOffPolicy = new ExponentialBackOffPolicy(); + backOffPolicy.setInitialInterval(initialBackoffMs); + backOffPolicy.setMultiplier(2.0); + backOffPolicy.setMaxInterval(maxBackoffMs); + template.setBackOffPolicy(backOffPolicy); + return template; + } + + private boolean isRetryableAwsErrorCode(String errorCode) { + if (errorCode == null) { + return false; + } + return switch (errorCode) { + case "SlowDown", "RequestTimeout", "InternalError", "ServiceUnavailable" -> true; + default -> false; + }; + } + + private Throwable rootCause(Throwable throwable) { + Throwable current = throwable; + while (current.getCause() != null && current.getCause() != current) { + current = current.getCause(); + } + return current; + } +} diff --git a/src/main/java/org/example/projektarendehantering/infrastructure/persistence/FailedS3DeletionEntity.java b/src/main/java/org/example/projektarendehantering/infrastructure/persistence/FailedS3DeletionEntity.java new file mode 100644 index 0000000..d3e4ba5 --- /dev/null +++ b/src/main/java/org/example/projektarendehantering/infrastructure/persistence/FailedS3DeletionEntity.java @@ -0,0 +1,42 @@ +package org.example.projektarendehantering.infrastructure.persistence; + +import jakarta.persistence.*; +import lombok.*; + +import java.time.Instant; +import java.util.UUID; + +@Getter +@Setter +@NoArgsConstructor +@AllArgsConstructor +@Builder +@Entity +@Table(name = "failed_s3_deletions") +public class FailedS3DeletionEntity { + + @Id + @GeneratedValue(strategy = GenerationType.UUID) + private UUID id; + + @Column(nullable = false) + private String bucket; + + @Column(nullable = false) + private String s3Key; + + @Column(nullable = false) + private int attemptCount; + + @Column(nullable = false) + private Instant nextAttemptAt; + + @Column(nullable = false) + private Instant createdAt; + + @Column(nullable = false) + private Instant updatedAt; + + @Column(length = 1000) + private String lastError; +} diff --git a/src/main/java/org/example/projektarendehantering/infrastructure/persistence/FailedS3DeletionRepository.java b/src/main/java/org/example/projektarendehantering/infrastructure/persistence/FailedS3DeletionRepository.java new file mode 100644 index 0000000..b0f2c7c --- /dev/null +++ b/src/main/java/org/example/projektarendehantering/infrastructure/persistence/FailedS3DeletionRepository.java @@ -0,0 +1,13 @@ +package org.example.projektarendehantering.infrastructure.persistence; + +import org.springframework.data.domain.Pageable; +import org.springframework.data.jpa.repository.JpaRepository; + +import java.time.Instant; +import java.util.List; +import java.util.UUID; + +public interface FailedS3DeletionRepository extends JpaRepository { + List findByNextAttemptAtBeforeOrderByCreatedAtAsc(Instant now, Pageable pageable); + boolean existsByBucketAndS3Key(String bucket, String s3Key); +} diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties index 007d45e..7128581 100644 --- a/src/main/resources/application.properties +++ b/src/main/resources/application.properties @@ -26,4 +26,12 @@ app.s3.bucket=documents # Multipart configuration spring.servlet.multipart.max-file-size=10MB -spring.servlet.multipart.max-request-size=10MB \ No newline at end of file +spring.servlet.multipart.max-request-size=10MB + +# S3 resilience configuration +app.s3.retry.max-attempts=3 +app.s3.retry.initial-backoff-ms=200 +app.s3.retry.max-backoff-ms=2000 +app.s3.failed-delete.batch-size=20 +app.s3.failed-delete.retry-delay-seconds=60 +app.s3.failed-delete.scheduler-delay-ms=30000 \ No newline at end of file diff --git a/src/test/java/org/example/projektarendehantering/application/service/DocumentServiceTest.java b/src/test/java/org/example/projektarendehantering/application/service/DocumentServiceTest.java index b59b564..5f99d2a 100644 --- a/src/test/java/org/example/projektarendehantering/application/service/DocumentServiceTest.java +++ b/src/test/java/org/example/projektarendehantering/application/service/DocumentServiceTest.java @@ -1,7 +1,9 @@ package org.example.projektarendehantering.application.service; import io.awspring.cloud.s3.ObjectMetadata; +import io.awspring.cloud.s3.S3Resource; import io.awspring.cloud.s3.S3Template; +import org.example.projektarendehantering.common.AppException; import org.example.projektarendehantering.common.Actor; import org.example.projektarendehantering.common.NotAuthorizedException; import org.example.projektarendehantering.common.Role; @@ -27,6 +29,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.*; @@ -41,6 +44,10 @@ class DocumentServiceTest { @Mock private S3Template s3Template; @Mock + private S3RetryExecutor s3RetryExecutor; + @Mock + private FailedS3DeletionService failedS3DeletionService; + @Mock private DocumentMapper documentMapper; @Mock private AuditService auditService; @@ -73,6 +80,7 @@ void setUp() { void uploadDocument_shouldAllowOwner() throws IOException { MockMultipartFile file = new MockMultipartFile("file", "test.txt", "text/plain", "hello".getBytes()); when(caseRepository.findById(caseId)).thenReturn(Optional.of(caseEntity)); + when(s3RetryExecutor.execute(eq("upload"), any())).thenReturn(null); when(documentRepository.save(any(DocumentEntity.class))).thenAnswer(i -> { DocumentEntity e = i.getArgument(0); e.setId(UUID.randomUUID()); @@ -84,7 +92,7 @@ void uploadDocument_shouldAllowOwner() throws IOException { assertThat(result).isNotNull(); assertThat(result.fileName()).isEqualTo("test.txt"); - verify(s3Template).upload(eq("test-bucket"), anyString(), any(InputStream.class), any(ObjectMetadata.class)); + verify(s3RetryExecutor).execute(eq("upload"), any()); verify(documentRepository).save(any(DocumentEntity.class)); } @@ -92,6 +100,7 @@ void uploadDocument_shouldAllowOwner() throws IOException { void uploadDocument_shouldAllowManager() throws IOException { MockMultipartFile file = new MockMultipartFile("file", "test.txt", "text/plain", "hello".getBytes()); when(caseRepository.findById(caseId)).thenReturn(Optional.of(caseEntity)); + when(s3RetryExecutor.execute(eq("upload"), any())).thenReturn(null); when(documentRepository.save(any(DocumentEntity.class))).thenAnswer(i -> { DocumentEntity e = i.getArgument(0); e.setId(UUID.randomUUID()); @@ -102,7 +111,7 @@ void uploadDocument_shouldAllowManager() throws IOException { DocumentDTO result = documentService.uploadDocument(managerActor, caseId, file); assertThat(result).isNotNull(); - verify(s3Template).upload(eq("test-bucket"), anyString(), any(InputStream.class), any(ObjectMetadata.class)); + verify(s3RetryExecutor).execute(eq("upload"), any()); } @Test @@ -123,11 +132,62 @@ void deleteDocument_shouldAllowOwner() { docEntity.setCaseEntity(caseEntity); docEntity.setS3Key("some-key"); + when(s3RetryExecutor.execute(eq("delete"), any())).thenReturn(null); + when(documentRepository.findById(docId)).thenReturn(Optional.of(docEntity)); + + documentService.deleteDocument(doctorActor, docId); + + verify(s3RetryExecutor).execute(eq("delete"), any()); + verify(documentRepository).delete(docEntity); + } + + @Test + void uploadDocument_shouldBubbleDegradedCodeWhenS3TransientlyUnavailable() { + MockMultipartFile file = new MockMultipartFile("file", "test.txt", "text/plain", "hello".getBytes()); + when(caseRepository.findById(caseId)).thenReturn(Optional.of(caseEntity)); + when(s3RetryExecutor.execute(eq("upload"), any())) + .thenThrow(new AppException("S3_SERVICE_DEGRADED", "Temporary S3 issue while trying to upload")); + + assertThatThrownBy(() -> documentService.uploadDocument(doctorActor, caseId, file)) + .isInstanceOf(AppException.class) + .hasMessageContaining("Temporary S3 issue") + .satisfies(ex -> assertThat(((AppException) ex).errorCode()).isEqualTo("S3_SERVICE_DEGRADED")); + } + + @Test + void downloadDocument_shouldUseRetryExecutor() { + UUID docId = UUID.randomUUID(); + DocumentEntity docEntity = new DocumentEntity(); + docEntity.setId(docId); + docEntity.setCaseEntity(caseEntity); + docEntity.setS3Key("download-key"); + S3Resource resource = mock(S3Resource.class); + + when(documentRepository.findById(docId)).thenReturn(Optional.of(docEntity)); + when(s3RetryExecutor.execute(eq("download"), any())).thenReturn(resource); + + S3Resource result = documentService.downloadDocument(doctorActor, docId); + + assertThat(result).isEqualTo(resource); + verify(s3RetryExecutor).execute(eq("download"), any()); + } + + @Test + void deleteDocument_shouldQueueFailedDeleteForLaterRecovery() { + UUID docId = UUID.randomUUID(); + DocumentEntity docEntity = new DocumentEntity(); + docEntity.setId(docId); + docEntity.setCaseEntity(caseEntity); + docEntity.setS3Key("failed-key"); + when(documentRepository.findById(docId)).thenReturn(Optional.of(docEntity)); + when(s3RetryExecutor.execute(eq("delete"), any())) + .thenThrow(new AppException("S3_SERVICE_DEGRADED", "delete degraded")); documentService.deleteDocument(doctorActor, docId); - verify(s3Template).deleteObject(eq("test-bucket"), eq("some-key")); + verify(failedS3DeletionService).enqueue(eq("test-bucket"), eq("failed-key"), argThat(ex -> + ex instanceof AppException && "S3_SERVICE_DEGRADED".equals(((AppException) ex).errorCode()))); verify(documentRepository).delete(docEntity); } } diff --git a/src/test/java/org/example/projektarendehantering/application/service/FailedS3DeletionServiceTest.java b/src/test/java/org/example/projektarendehantering/application/service/FailedS3DeletionServiceTest.java new file mode 100644 index 0000000..abd033e --- /dev/null +++ b/src/test/java/org/example/projektarendehantering/application/service/FailedS3DeletionServiceTest.java @@ -0,0 +1,104 @@ +package org.example.projektarendehantering.application.service; + +import io.awspring.cloud.s3.S3Template; +import org.example.projektarendehantering.common.AppException; +import org.example.projektarendehantering.infrastructure.persistence.FailedS3DeletionEntity; +import org.example.projektarendehantering.infrastructure.persistence.FailedS3DeletionRepository; +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.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.data.domain.Pageable; +import org.springframework.test.util.ReflectionTestUtils; + +import java.time.Instant; +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.*; + +@ExtendWith(MockitoExtension.class) +class FailedS3DeletionServiceTest { + + @Mock + private FailedS3DeletionRepository failedS3DeletionRepository; + @Mock + private S3Template s3Template; + @Mock + private S3RetryExecutor s3RetryExecutor; + @Mock + private AuditService auditService; + + @InjectMocks + private FailedS3DeletionService service; + + @BeforeEach + void setUp() { + ReflectionTestUtils.setField(service, "batchSize", 10); + ReflectionTestUtils.setField(service, "retryDelaySeconds", 1L); + } + + @Test + void enqueue_shouldPersistFailedDeletion() { + when(failedS3DeletionRepository.existsByBucketAndS3Key(anyString(), anyString())).thenReturn(false); + service.enqueue("bucket-a", "key-a", new RuntimeException("boom")); + + ArgumentCaptor captor = ArgumentCaptor.forClass(FailedS3DeletionEntity.class); + verify(failedS3DeletionRepository).save(captor.capture()); + FailedS3DeletionEntity saved = captor.getValue(); + assertThat(saved.getBucket()).isEqualTo("bucket-a"); + assertThat(saved.getS3Key()).isEqualTo("key-a"); + assertThat(saved.getAttemptCount()).isZero(); + assertThat(saved.getLastError()).contains("boom"); + assertThat(saved.getNextAttemptAt()).isAfterOrEqualTo(Instant.now().minusSeconds(1)); + } + + @Test + void processPendingDeletions_shouldDeleteEntryWhenRetrySucceeds() { + FailedS3DeletionEntity pending = FailedS3DeletionEntity.builder() + .bucket("bucket-a") + .s3Key("key-a") + .attemptCount(0) + .createdAt(Instant.now()) + .updatedAt(Instant.now()) + .nextAttemptAt(Instant.now().minusSeconds(1)) + .build(); + + when(failedS3DeletionRepository.findByNextAttemptAtBeforeOrderByCreatedAtAsc(any(), any(Pageable.class))) + .thenReturn(List.of(pending)); + when(s3RetryExecutor.execute(eq("delete"), any())).thenReturn(null); + + service.processPendingDeletions(); + + verify(failedS3DeletionRepository).delete(pending); + } + + @Test + void processPendingDeletions_shouldRescheduleWhenRetryFails() { + FailedS3DeletionEntity pending = FailedS3DeletionEntity.builder() + .bucket("bucket-a") + .s3Key("key-a") + .attemptCount(1) + .createdAt(Instant.now()) + .updatedAt(Instant.now()) + .nextAttemptAt(Instant.now().minusSeconds(1)) + .build(); + + when(failedS3DeletionRepository.findByNextAttemptAtBeforeOrderByCreatedAtAsc(any(), any(Pageable.class))) + .thenReturn(List.of(pending)); + when(s3RetryExecutor.execute(eq("delete"), any())) + .thenThrow(new AppException("S3_SERVICE_DEGRADED", "temporary issue")); + + service.processPendingDeletions(); + + verify(failedS3DeletionRepository, atLeastOnce()).save(pending); + assertThat(pending.getAttemptCount()).isEqualTo(2); + assertThat(pending.getLastError()).contains("temporary issue"); + } +} diff --git a/src/test/java/org/example/projektarendehantering/application/service/S3RetryExecutorTest.java b/src/test/java/org/example/projektarendehantering/application/service/S3RetryExecutorTest.java new file mode 100644 index 0000000..efb5561 --- /dev/null +++ b/src/test/java/org/example/projektarendehantering/application/service/S3RetryExecutorTest.java @@ -0,0 +1,53 @@ +package org.example.projektarendehantering.application.service; + +import org.example.projektarendehantering.common.AppException; +import org.junit.jupiter.api.Test; +import software.amazon.awssdk.core.exception.SdkClientException; +import software.amazon.awssdk.services.s3.model.S3Exception; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +class S3RetryExecutorTest { + + private final S3RetryExecutor executor = new S3RetryExecutor(3, 1, 5); + + @Test + void execute_shouldRetryAndSucceedForTransientFailures() { + int[] attempts = {0}; + + String result = executor.execute("upload", context -> { + attempts[0]++; + if (attempts[0] < 3) { + throw S3Exception.builder().statusCode(503).message("ServiceUnavailable").build(); + } + return "ok"; + }); + + assertThat(result).isEqualTo("ok"); + assertThat(attempts[0]).isEqualTo(3); + } + + @Test + void execute_shouldReturnDegradedCodeWhenTransientFailurePersists() { + assertThatThrownBy(() -> executor.execute("download", context -> { + throw SdkClientException.builder().message("Connection reset").build(); + })) + .isInstanceOf(AppException.class) + .satisfies(ex -> assertThat(((AppException) ex).errorCode()).isEqualTo("S3_SERVICE_DEGRADED")); + } + + @Test + void execute_shouldNotRetryPermanentFailures() { + int[] attempts = {0}; + + assertThatThrownBy(() -> executor.execute("delete", context -> { + attempts[0]++; + throw S3Exception.builder().statusCode(403).message("AccessDenied").build(); + })) + .isInstanceOf(AppException.class) + .satisfies(ex -> assertThat(((AppException) ex).errorCode()).isEqualTo("S3_DELETE_FAILED")); + + assertThat(attempts[0]).isEqualTo(1); + } +} diff --git a/src/test/resources/application.properties b/src/test/resources/application.properties index b007820..048248f 100644 --- a/src/test/resources/application.properties +++ b/src/test/resources/application.properties @@ -31,4 +31,10 @@ spring.cloud.aws.credentials.access-key=minioadmin spring.cloud.aws.credentials.secret-key=minioadmin spring.cloud.aws.region.static=us-east-1 spring.cloud.aws.s3.path-style-access-enabled=true -app.s3.bucket=test-documents \ No newline at end of file +app.s3.bucket=test-documents +app.s3.retry.max-attempts=2 +app.s3.retry.initial-backoff-ms=1 +app.s3.retry.max-backoff-ms=5 +app.s3.failed-delete.batch-size=10 +app.s3.failed-delete.retry-delay-seconds=1 +app.s3.failed-delete.scheduler-delay-ms=1000 \ No newline at end of file From a6d5018b7de2d37ac60fbb0188cfe3757671cd65 Mon Sep 17 00:00:00 2001 From: Linus Westling Date: Thu, 23 Apr 2026 09:10:46 +0200 Subject: [PATCH 2/7] rabbit feedback fixes --- .../application/service/AuditService.java | 3 +- .../service/FailedS3DeletionService.java | 109 +++++-- .../application/service/S3RetryExecutor.java | 13 +- .../persistence/FailedS3DeletionEntity.java | 6 +- src/main/resources/application.properties | 1 + .../resources/templates/fragments/head.html | 290 +++++++++++++++++- .../service/FailedS3DeletionServiceTest.java | 44 ++- src/test/resources/application.properties | 1 + 8 files changed, 431 insertions(+), 36 deletions(-) diff --git a/src/main/java/org/example/projektarendehantering/application/service/AuditService.java b/src/main/java/org/example/projektarendehantering/application/service/AuditService.java index 65ed3aa..04d110c 100644 --- a/src/main/java/org/example/projektarendehantering/application/service/AuditService.java +++ b/src/main/java/org/example/projektarendehantering/application/service/AuditService.java @@ -15,6 +15,7 @@ import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Propagation; import org.springframework.transaction.annotation.Transactional; import org.springframework.transaction.support.TransactionSynchronization; import org.springframework.transaction.support.TransactionSynchronizationManager; @@ -67,7 +68,7 @@ public class AuditService { "refresh_token" ); - @Transactional + @Transactional(propagation = Propagation.REQUIRES_NEW) public void record(AuditEventEntity event) { if (event == null) return; if (event.getId() == null) { diff --git a/src/main/java/org/example/projektarendehantering/application/service/FailedS3DeletionService.java b/src/main/java/org/example/projektarendehantering/application/service/FailedS3DeletionService.java index 141ddc2..b036703 100644 --- a/src/main/java/org/example/projektarendehantering/application/service/FailedS3DeletionService.java +++ b/src/main/java/org/example/projektarendehantering/application/service/FailedS3DeletionService.java @@ -6,15 +6,18 @@ import org.example.projektarendehantering.infrastructure.persistence.FailedS3DeletionEntity; import org.example.projektarendehantering.infrastructure.persistence.FailedS3DeletionRepository; import org.example.projektarendehantering.infrastructure.persistence.AuditEventEntity; +import org.springframework.beans.factory.ObjectProvider; import org.springframework.beans.factory.annotation.Value; import org.springframework.data.domain.PageRequest; import org.springframework.scheduling.annotation.Scheduled; import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Propagation; import org.springframework.transaction.annotation.Transactional; import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.List; +import java.util.UUID; @Slf4j @Service @@ -25,6 +28,7 @@ public class FailedS3DeletionService { private final S3Template s3Template; private final S3RetryExecutor s3RetryExecutor; private final AuditService auditService; + private final ObjectProvider selfProvider; @Value("${app.s3.failed-delete.batch-size:20}") private int batchSize; @@ -32,13 +36,17 @@ public class FailedS3DeletionService { @Value("${app.s3.failed-delete.retry-delay-seconds:60}") private long retryDelaySeconds; - @Transactional + @Value("${app.s3.failed-delete.max-attempts:10}") + private int maxAttempts; + + @Transactional(propagation = Propagation.REQUIRES_NEW) public void enqueue(String bucket, String s3Key, Exception e) { if (failedS3DeletionRepository.existsByBucketAndS3Key(bucket, s3Key)) { log.warn("Failed S3 deletion already queued. bucket={}, key={}", bucket, s3Key); return; } Instant now = Instant.now(); + String trimmed = trimError(e); FailedS3DeletionEntity entity = FailedS3DeletionEntity.builder() .bucket(bucket) .s3Key(s3Key) @@ -46,57 +54,96 @@ public void enqueue(String bucket, String s3Key, Exception e) { .nextAttemptAt(now.plus(retryDelaySeconds, ChronoUnit.SECONDS)) .createdAt(now) .updatedAt(now) - .lastError(trimError(e)) + .lastError(trimmed) .build(); failedS3DeletionRepository.save(entity); log.warn("Queued failed S3 deletion for retry. bucket={}, key={}", bucket, s3Key); auditService.record(AuditEventEntity.builder() .eventName("DOCUMENT_S3_DELETE_QUEUED") .description("Queued failed S3 deletion for retry") - .queryString("bucket=" + bucket + "&s3Key=" + s3Key + "&error=" + trimError(e)) + .queryString("bucket=" + bucket + "&s3Key=" + s3Key + "&error=" + trimmed) .occurredAt(now) .build()); } @Scheduled(fixedDelayString = "${app.s3.failed-delete.scheduler-delay-ms:30000}") - @Transactional public void processPendingDeletions() { List items = failedS3DeletionRepository .findByNextAttemptAtBeforeOrderByCreatedAtAsc(Instant.now(), PageRequest.of(0, batchSize)); for (FailedS3DeletionEntity item : items) { - try { - s3RetryExecutor.execute("delete", context -> { - s3Template.deleteObject(item.getBucket(), item.getS3Key()); - return null; - }); - failedS3DeletionRepository.delete(item); - log.info("Recovered failed S3 deletion. bucket={}, key={}", item.getBucket(), item.getS3Key()); - auditService.record(AuditEventEntity.builder() - .eventName("DOCUMENT_S3_DELETE_RECOVERED") - .description("Recovered failed S3 deletion from retry queue") - .queryString("bucket=" + item.getBucket() + "&s3Key=" + item.getS3Key() + "&attempts=" + item.getAttemptCount()) - .occurredAt(Instant.now()) - .build()); - } catch (Exception ex) { - item.setAttemptCount(item.getAttemptCount() + 1); - item.setUpdatedAt(Instant.now()); - item.setNextAttemptAt(Instant.now().plus(retryDelaySeconds, ChronoUnit.SECONDS)); - item.setLastError(trimError(ex)); - failedS3DeletionRepository.save(item); - log.warn("Failed retrying S3 deletion. bucket={}, key={}, attempts={}", + selfProvider.getObject().processOne(item.getId()); + } + } + + @Transactional(propagation = Propagation.REQUIRES_NEW) + public void processOne(UUID id) { + FailedS3DeletionEntity item = failedS3DeletionRepository.findById(id).orElse(null); + if (item == null) { + return; + } + if (item.getAttemptCount() >= maxAttempts) { + markDeadLetter(item, "max attempts exceeded"); + return; + } + try { + s3RetryExecutor.execute("delete", context -> { + s3Template.deleteObject(item.getBucket(), item.getS3Key()); + return null; + }); + failedS3DeletionRepository.delete(item); + log.info("Recovered failed S3 deletion. bucket={}, key={}", item.getBucket(), item.getS3Key()); + auditService.record(AuditEventEntity.builder() + .eventName("DOCUMENT_S3_DELETE_RECOVERED") + .description("Recovered failed S3 deletion from retry queue") + .queryString("bucket=" + item.getBucket() + "&s3Key=" + item.getS3Key() + "&attempts=" + item.getAttemptCount()) + .occurredAt(Instant.now()) + .build()); + } catch (Exception ex) { + int nextAttemptCount = item.getAttemptCount() + 1; + item.setAttemptCount(nextAttemptCount); + item.setUpdatedAt(Instant.now()); + item.setLastError(trimError(ex)); + + if (nextAttemptCount >= maxAttempts) { + markDeadLetter(item, trimError(ex)); + log.warn("Giving up failed S3 deletion after max attempts. bucket={}, key={}, attempts={}", item.getBucket(), item.getS3Key(), item.getAttemptCount(), ex); - auditService.record(AuditEventEntity.builder() - .eventName("DOCUMENT_S3_DELETE_RETRY_FAILED") - .description("Retry failed for queued S3 deletion") - .queryString("bucket=" + item.getBucket() + "&s3Key=" + item.getS3Key() - + "&attempts=" + item.getAttemptCount() + "&error=" + trimError(ex)) - .occurredAt(Instant.now()) - .build()); + return; } + + long delaySeconds = computeRetryDelaySeconds(nextAttemptCount); + item.setNextAttemptAt(Instant.now().plus(delaySeconds, ChronoUnit.SECONDS)); + failedS3DeletionRepository.save(item); + log.warn("Failed retrying S3 deletion. bucket={}, key={}, attempts={}", + item.getBucket(), item.getS3Key(), item.getAttemptCount(), ex); + auditService.record(AuditEventEntity.builder() + .eventName("DOCUMENT_S3_DELETE_RETRY_FAILED") + .description("Retry failed for queued S3 deletion") + .queryString("bucket=" + item.getBucket() + "&s3Key=" + item.getS3Key() + + "&attempts=" + item.getAttemptCount() + "&error=" + trimError(ex)) + .occurredAt(Instant.now()) + .build()); } } + private long computeRetryDelaySeconds(int attemptCount) { + long baseDelay = Math.max(1L, retryDelaySeconds); + long multiplier = Math.min(Math.max(1, attemptCount), 10); + return baseDelay * multiplier; + } + + private void markDeadLetter(FailedS3DeletionEntity item, String reason) { + failedS3DeletionRepository.delete(item); + auditService.record(AuditEventEntity.builder() + .eventName("DOCUMENT_S3_DELETE_DEAD_LETTERED") + .description("Queued S3 deletion reached max retry attempts and was dead-lettered") + .queryString("bucket=" + item.getBucket() + "&s3Key=" + item.getS3Key() + + "&attempts=" + item.getAttemptCount() + "&reason=" + reason) + .occurredAt(Instant.now()) + .build()); + } + private String trimError(Exception e) { String message = e.getMessage(); if (message == null) { diff --git a/src/main/java/org/example/projektarendehantering/application/service/S3RetryExecutor.java b/src/main/java/org/example/projektarendehantering/application/service/S3RetryExecutor.java index 0f3f143..0db6d3b 100644 --- a/src/main/java/org/example/projektarendehantering/application/service/S3RetryExecutor.java +++ b/src/main/java/org/example/projektarendehantering/application/service/S3RetryExecutor.java @@ -11,6 +11,8 @@ import org.springframework.retry.support.RetryTemplate; import org.springframework.stereotype.Component; import software.amazon.awssdk.awscore.exception.AwsServiceException; +import software.amazon.awssdk.core.exception.NonRetryableException; +import software.amazon.awssdk.core.exception.RetryableException; import software.amazon.awssdk.core.exception.SdkClientException; import software.amazon.awssdk.services.s3.model.S3Exception; @@ -51,7 +53,16 @@ public boolean isRetryable(Throwable throwable) { if (root instanceof AwsServiceException awsServiceException) { return RETRYABLE_HTTP_STATUS.contains(awsServiceException.statusCode()); } - return root instanceof SdkClientException; + if (root instanceof NonRetryableException) { + return false; + } + if (root instanceof RetryableException) { + return true; + } + if (root instanceof SdkClientException sdkClientException) { + return sdkClientException.retryable(); + } + return false; } private AppException mapToAppException(String operationName, RuntimeException ex) { diff --git a/src/main/java/org/example/projektarendehantering/infrastructure/persistence/FailedS3DeletionEntity.java b/src/main/java/org/example/projektarendehantering/infrastructure/persistence/FailedS3DeletionEntity.java index d3e4ba5..8606923 100644 --- a/src/main/java/org/example/projektarendehantering/infrastructure/persistence/FailedS3DeletionEntity.java +++ b/src/main/java/org/example/projektarendehantering/infrastructure/persistence/FailedS3DeletionEntity.java @@ -12,7 +12,11 @@ @AllArgsConstructor @Builder @Entity -@Table(name = "failed_s3_deletions") +@Table( + name = "failed_s3_deletions", + uniqueConstraints = @UniqueConstraint(columnNames = {"bucket", "s3Key"}), + indexes = @Index(name = "idx_failed_s3_next_attempt", columnList = "nextAttemptAt,createdAt") +) public class FailedS3DeletionEntity { @Id diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties index 7128581..5f5b8dc 100644 --- a/src/main/resources/application.properties +++ b/src/main/resources/application.properties @@ -34,4 +34,5 @@ app.s3.retry.initial-backoff-ms=200 app.s3.retry.max-backoff-ms=2000 app.s3.failed-delete.batch-size=20 app.s3.failed-delete.retry-delay-seconds=60 +app.s3.failed-delete.max-attempts=10 app.s3.failed-delete.scheduler-delay-ms=30000 \ No newline at end of file diff --git a/src/main/resources/templates/fragments/head.html b/src/main/resources/templates/fragments/head.html index 798a549..50865c8 100644 --- a/src/main/resources/templates/fragments/head.html +++ b/src/main/resources/templates/fragments/head.html @@ -4,6 +4,294 @@ Ärendehantering - + diff --git a/src/test/java/org/example/projektarendehantering/application/service/FailedS3DeletionServiceTest.java b/src/test/java/org/example/projektarendehantering/application/service/FailedS3DeletionServiceTest.java index abd033e..e9aefbc 100644 --- a/src/test/java/org/example/projektarendehantering/application/service/FailedS3DeletionServiceTest.java +++ b/src/test/java/org/example/projektarendehantering/application/service/FailedS3DeletionServiceTest.java @@ -11,11 +11,15 @@ import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.beans.factory.ObjectProvider; import org.springframework.data.domain.Pageable; +import org.springframework.retry.RetryCallback; import org.springframework.test.util.ReflectionTestUtils; import java.time.Instant; import java.util.List; +import java.util.Optional; +import java.util.UUID; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; @@ -34,6 +38,8 @@ class FailedS3DeletionServiceTest { private S3RetryExecutor s3RetryExecutor; @Mock private AuditService auditService; + @Mock + private ObjectProvider selfProvider; @InjectMocks private FailedS3DeletionService service; @@ -42,6 +48,7 @@ class FailedS3DeletionServiceTest { void setUp() { ReflectionTestUtils.setField(service, "batchSize", 10); ReflectionTestUtils.setField(service, "retryDelaySeconds", 1L); + ReflectionTestUtils.setField(service, "maxAttempts", 3); } @Test @@ -62,6 +69,7 @@ void enqueue_shouldPersistFailedDeletion() { @Test void processPendingDeletions_shouldDeleteEntryWhenRetrySucceeds() { FailedS3DeletionEntity pending = FailedS3DeletionEntity.builder() + .id(UUID.randomUUID()) .bucket("bucket-a") .s3Key("key-a") .attemptCount(0) @@ -72,16 +80,23 @@ void processPendingDeletions_shouldDeleteEntryWhenRetrySucceeds() { when(failedS3DeletionRepository.findByNextAttemptAtBeforeOrderByCreatedAtAsc(any(), any(Pageable.class))) .thenReturn(List.of(pending)); - when(s3RetryExecutor.execute(eq("delete"), any())).thenReturn(null); + when(selfProvider.getObject()).thenReturn(service); + when(failedS3DeletionRepository.findById(pending.getId())).thenReturn(Optional.of(pending)); + when(s3RetryExecutor.execute(eq("delete"), any())).thenAnswer(invocation -> { + RetryCallback callback = invocation.getArgument(1); + return callback.doWithRetry(null); + }); service.processPendingDeletions(); + verify(s3Template).deleteObject("bucket-a", "key-a"); verify(failedS3DeletionRepository).delete(pending); } @Test void processPendingDeletions_shouldRescheduleWhenRetryFails() { FailedS3DeletionEntity pending = FailedS3DeletionEntity.builder() + .id(UUID.randomUUID()) .bucket("bucket-a") .s3Key("key-a") .attemptCount(1) @@ -92,6 +107,8 @@ void processPendingDeletions_shouldRescheduleWhenRetryFails() { when(failedS3DeletionRepository.findByNextAttemptAtBeforeOrderByCreatedAtAsc(any(), any(Pageable.class))) .thenReturn(List.of(pending)); + when(selfProvider.getObject()).thenReturn(service); + when(failedS3DeletionRepository.findById(pending.getId())).thenReturn(Optional.of(pending)); when(s3RetryExecutor.execute(eq("delete"), any())) .thenThrow(new AppException("S3_SERVICE_DEGRADED", "temporary issue")); @@ -101,4 +118,29 @@ void processPendingDeletions_shouldRescheduleWhenRetryFails() { assertThat(pending.getAttemptCount()).isEqualTo(2); assertThat(pending.getLastError()).contains("temporary issue"); } + + @Test + void processPendingDeletions_shouldDeadLetterWhenMaxAttemptsReached() { + FailedS3DeletionEntity pending = FailedS3DeletionEntity.builder() + .id(UUID.randomUUID()) + .bucket("bucket-a") + .s3Key("key-a") + .attemptCount(2) + .createdAt(Instant.now()) + .updatedAt(Instant.now()) + .nextAttemptAt(Instant.now().minusSeconds(1)) + .build(); + + when(failedS3DeletionRepository.findByNextAttemptAtBeforeOrderByCreatedAtAsc(any(), any(Pageable.class))) + .thenReturn(List.of(pending)); + when(selfProvider.getObject()).thenReturn(service); + when(failedS3DeletionRepository.findById(pending.getId())).thenReturn(Optional.of(pending)); + when(s3RetryExecutor.execute(eq("delete"), any())) + .thenThrow(new AppException("S3_SERVICE_DEGRADED", "temporary issue")); + + service.processPendingDeletions(); + + verify(failedS3DeletionRepository).delete(pending); + verify(failedS3DeletionRepository, never()).save(pending); + } } diff --git a/src/test/resources/application.properties b/src/test/resources/application.properties index 048248f..ad40a60 100644 --- a/src/test/resources/application.properties +++ b/src/test/resources/application.properties @@ -37,4 +37,5 @@ app.s3.retry.initial-backoff-ms=1 app.s3.retry.max-backoff-ms=5 app.s3.failed-delete.batch-size=10 app.s3.failed-delete.retry-delay-seconds=1 +app.s3.failed-delete.max-attempts=3 app.s3.failed-delete.scheduler-delay-ms=1000 \ No newline at end of file From dd72b77812c7d879797137e66188c5a16666523b Mon Sep 17 00:00:00 2001 From: Linus Westling Date: Thu, 23 Apr 2026 09:27:54 +0200 Subject: [PATCH 3/7] Fixed test errors and additional rabbit feedback about s3 handling --- AUDIT.md | 2 +- .../service/FailedS3DeletionService.java | 6 +- .../application/service/S3RetryExecutor.java | 46 ++- .../config/AuditWebMvcConfig.java | 1 - .../infrastructure/config/SecurityConfig.java | 2 +- .../persistence/FailedS3DeletionEntity.java | 4 +- src/main/resources/static/app.css | 270 ------------------ 7 files changed, 50 insertions(+), 281 deletions(-) delete mode 100644 src/main/resources/static/app.css diff --git a/AUDIT.md b/AUDIT.md index 6e8663c..9469b32 100644 --- a/AUDIT.md +++ b/AUDIT.md @@ -59,7 +59,7 @@ Table **`audit_events`** (entity `AuditEventEntity`): `AuditWebMvcConfig` adds `AuditInterceptor` for: - **Included:** `/ui/**`, `/api/**` -- **Excluded:** `/static/**`, `/app.css`, `/app.js`, `/error**`, `/login**` +- **Excluded:** `/static/**`, `/app.js`, `/error**`, `/login**` So static assets, error pages, and login flows do not generate audit rows. diff --git a/src/main/java/org/example/projektarendehantering/application/service/FailedS3DeletionService.java b/src/main/java/org/example/projektarendehantering/application/service/FailedS3DeletionService.java index b036703..d6d6aa9 100644 --- a/src/main/java/org/example/projektarendehantering/application/service/FailedS3DeletionService.java +++ b/src/main/java/org/example/projektarendehantering/application/service/FailedS3DeletionService.java @@ -72,7 +72,11 @@ public void processPendingDeletions() { .findByNextAttemptAtBeforeOrderByCreatedAtAsc(Instant.now(), PageRequest.of(0, batchSize)); for (FailedS3DeletionEntity item : items) { - selfProvider.getObject().processOne(item.getId()); + try { + selfProvider.getObject().processOne(item.getId()); + } catch (Exception ex) { + log.error("Unexpected error while processing failed S3 deletion item. id={}", item.getId(), ex); + } } } diff --git a/src/main/java/org/example/projektarendehantering/application/service/S3RetryExecutor.java b/src/main/java/org/example/projektarendehantering/application/service/S3RetryExecutor.java index 0db6d3b..e70f2c0 100644 --- a/src/main/java/org/example/projektarendehantering/application/service/S3RetryExecutor.java +++ b/src/main/java/org/example/projektarendehantering/application/service/S3RetryExecutor.java @@ -23,6 +23,15 @@ public class S3RetryExecutor { private static final Logger log = LoggerFactory.getLogger(S3RetryExecutor.class); private static final Set RETRYABLE_HTTP_STATUS = Set.of(408, 429, 500, 502, 503, 504); + private static final String[] TRANSIENT_CLIENT_ERROR_MARKERS = { + "connection reset", + "connection refused", + "timed out", + "timeout", + "broken pipe", + "i/o", + "eof" + }; private final RetryTemplate retryTemplate; @@ -38,6 +47,12 @@ public T execute(String operationName, RetryCallback ca try { return retryTemplate.execute(callback); } catch (RuntimeException ex) { + if (ex instanceof AppException appException) { + throw appException; + } + if (ex.getCause() instanceof AppException appExceptionCause) { + throw appExceptionCause; + } throw mapToAppException(operationName, ex); } } @@ -46,12 +61,16 @@ public boolean isRetryable(Throwable throwable) { Throwable root = rootCause(throwable); if (root instanceof S3Exception s3Exception) { - return RETRYABLE_HTTP_STATUS.contains(s3Exception.statusCode()) || isRetryableAwsErrorCode(s3Exception.awsErrorDetails() != null - ? s3Exception.awsErrorDetails().errorCode() - : null); + return isRetryableStatusOrAwsError( + s3Exception.statusCode(), + s3Exception.awsErrorDetails() != null ? s3Exception.awsErrorDetails().errorCode() : null + ); } if (root instanceof AwsServiceException awsServiceException) { - return RETRYABLE_HTTP_STATUS.contains(awsServiceException.statusCode()); + return isRetryableStatusOrAwsError( + awsServiceException.statusCode(), + awsServiceException.awsErrorDetails() != null ? awsServiceException.awsErrorDetails().errorCode() : null + ); } if (root instanceof NonRetryableException) { return false; @@ -60,7 +79,7 @@ public boolean isRetryable(Throwable throwable) { return true; } if (root instanceof SdkClientException sdkClientException) { - return sdkClientException.retryable(); + return sdkClientException.retryable() || isLikelyTransientClientError(sdkClientException.getMessage()); } return false; } @@ -109,6 +128,10 @@ private boolean isRetryableAwsErrorCode(String errorCode) { }; } + private boolean isRetryableStatusOrAwsError(int statusCode, String awsErrorCode) { + return RETRYABLE_HTTP_STATUS.contains(statusCode) || isRetryableAwsErrorCode(awsErrorCode); + } + private Throwable rootCause(Throwable throwable) { Throwable current = throwable; while (current.getCause() != null && current.getCause() != current) { @@ -116,4 +139,17 @@ private Throwable rootCause(Throwable throwable) { } return current; } + + private boolean isLikelyTransientClientError(String message) { + if (message == null || message.isBlank()) { + return false; + } + String normalized = message.toLowerCase(); + for (String marker : TRANSIENT_CLIENT_ERROR_MARKERS) { + if (normalized.contains(marker)) { + return true; + } + } + return false; + } } diff --git a/src/main/java/org/example/projektarendehantering/infrastructure/config/AuditWebMvcConfig.java b/src/main/java/org/example/projektarendehantering/infrastructure/config/AuditWebMvcConfig.java index 7f543ac..981a67d 100644 --- a/src/main/java/org/example/projektarendehantering/infrastructure/config/AuditWebMvcConfig.java +++ b/src/main/java/org/example/projektarendehantering/infrastructure/config/AuditWebMvcConfig.java @@ -20,7 +20,6 @@ public void addInterceptors(InterceptorRegistry registry) { .addPathPatterns("/ui/**", "/api/**") .excludePathPatterns( "/static/**", - "/app.css", "/app.js", "/error**", "/login**" diff --git a/src/main/java/org/example/projektarendehantering/infrastructure/config/SecurityConfig.java b/src/main/java/org/example/projektarendehantering/infrastructure/config/SecurityConfig.java index cf17a12..7049f23 100644 --- a/src/main/java/org/example/projektarendehantering/infrastructure/config/SecurityConfig.java +++ b/src/main/java/org/example/projektarendehantering/infrastructure/config/SecurityConfig.java @@ -34,7 +34,7 @@ public SecurityConfig(LocalUserDetailsService localUserDetailsService) { public SecurityFilterChain securityFilterChain(HttpSecurity http, CustomOAuth2UserService customOAuth2UserService) throws Exception { http .authorizeHttpRequests(authorize -> authorize - .requestMatchers("/login**", "/register", "/error**", "/static/**", "/app.css", "/app.js", "/webjars/**").permitAll() + .requestMatchers("/login**", "/register", "/error**", "/static/**", "/app.js", "/webjars/**").permitAll() .anyRequest().authenticated() ) .oauth2Login(oauth2 -> oauth2 diff --git a/src/main/java/org/example/projektarendehantering/infrastructure/persistence/FailedS3DeletionEntity.java b/src/main/java/org/example/projektarendehantering/infrastructure/persistence/FailedS3DeletionEntity.java index 8606923..8ff4d1d 100644 --- a/src/main/java/org/example/projektarendehantering/infrastructure/persistence/FailedS3DeletionEntity.java +++ b/src/main/java/org/example/projektarendehantering/infrastructure/persistence/FailedS3DeletionEntity.java @@ -14,8 +14,8 @@ @Entity @Table( name = "failed_s3_deletions", - uniqueConstraints = @UniqueConstraint(columnNames = {"bucket", "s3Key"}), - indexes = @Index(name = "idx_failed_s3_next_attempt", columnList = "nextAttemptAt,createdAt") + uniqueConstraints = @UniqueConstraint(columnNames = {"bucket", "s3_key"}), + indexes = @Index(name = "idx_failed_s3_next_attempt", columnList = "next_attempt_at,created_at") ) public class FailedS3DeletionEntity { diff --git a/src/main/resources/static/app.css b/src/main/resources/static/app.css deleted file mode 100644 index 28dd506..0000000 --- a/src/main/resources/static/app.css +++ /dev/null @@ -1,270 +0,0 @@ -:root { - --bg: #0b1020; - --panel: #101a33; - --panel2: #0f1730; - --text: #e7ecff; - --muted: rgba(231, 236, 255, 0.7); - --border: rgba(231, 236, 255, 0.12); - --accent: #6ea8ff; - --accent-strong: #8fbeff; - --success: #7be495; - --danger: #ff6e8a; - --shadow: 0 10px 30px rgba(0, 0, 0, 0.35); - --radius: 16px; - --transition-fast: 160ms ease; -} - -* { box-sizing: border-box; } -html, body { height: 100%; } -body { - margin: 0; - font-family: ui-sans-serif, system-ui, -apple-system, Segoe UI, Roboto, Arial, "Apple Color Emoji", "Segoe UI Emoji"; - background: radial-gradient(1200px 700px at 20% 0%, rgba(110, 168, 255, 0.25), transparent 60%), - radial-gradient(900px 500px at 70% 20%, rgba(142, 97, 255, 0.18), transparent 60%), - var(--bg); - color: var(--text); - line-height: 1.5; -} - -code, pre { font-family: ui-monospace, SFMono-Regular, Menlo, Monaco, Consolas, "Liberation Mono", "Courier New", monospace; } -code { color: rgba(231, 236, 255, 0.95); } -a { color: inherit; } - -.container { - width: min(1140px, calc(100% - 40px)); - margin: 0 auto; -} - -.app-header { - position: sticky; - top: 0; - z-index: 10; - background: rgba(11, 16, 32, 0.75); - backdrop-filter: blur(10px); - border-bottom: 1px solid var(--border); -} - -.header-row { - display: grid; - grid-template-columns: 1fr auto auto; - gap: 20px; - align-items: center; - padding: 14px 0 12px; -} - -.brand-link { - color: var(--text); - text-decoration: none; - font-weight: 700; - letter-spacing: 0.25px; - font-size: 18px; -} - -.nav { display: flex; gap: 8px; flex-wrap: wrap; } -.nav-link { - color: var(--muted); - text-decoration: none; - padding: 8px 10px; - border-radius: 10px; - border: 1px solid transparent; - transition: all var(--transition-fast); -} -.nav-link:hover { - color: var(--text); - border-color: var(--border); - background: rgba(255, 255, 255, 0.03); -} - -.auth { - display: flex; - gap: 10px; - align-items: center; -} -.auth-label { font-size: 14px; color: var(--muted); margin-right: 10px; } - -.page { padding: 30px 0 46px; } -.page-title { display: flex; justify-content: space-between; align-items: center; gap: 12px; } -h1 { margin: 0 0 12px; font-size: clamp(28px, 4vw, 34px); line-height: 1.18; letter-spacing: 0.1px; } -h2 { margin: 0 0 10px; font-size: 20px; line-height: 1.3; } -h3 { margin: 0 0 8px; font-size: 16px; line-height: 1.35; } -h4 { margin: 0 0 8px; font-size: 16px; line-height: 1.35; } -p { margin: 0 0 14px; color: var(--muted); max-width: 75ch; } -.muted { color: var(--muted); } - -.cards { - display: grid; - grid-template-columns: repeat(auto-fit, minmax(240px, 1fr)); - gap: 16px; - margin-top: 20px; -} -.card { - display: block; - text-decoration: none; - color: var(--text); - background: linear-gradient(180deg, rgba(255,255,255,0.04), rgba(255,255,255,0.02)); - border: 1px solid var(--border); - border-radius: var(--radius); - padding: 18px; - box-shadow: var(--shadow); - transition: transform var(--transition-fast), border-color var(--transition-fast), background var(--transition-fast); -} -.card:hover { - border-color: rgba(110, 168, 255, 0.45); - background: linear-gradient(180deg, rgba(255,255,255,0.06), rgba(255,255,255,0.03)); - transform: translateY(-1px); -} -.card p { margin: 6px 0 0; } - -.panel { - background: linear-gradient(180deg, rgba(255,255,255,0.04), rgba(255,255,255,0.02)); - border: 1px solid var(--border); - border-radius: var(--radius); - padding: 18px; - box-shadow: var(--shadow); -} -.panel + .panel { margin-top: 14px; } -.panel-error { border-color: rgba(255, 110, 138, 0.35); } -.panel-notice { border-color: rgba(110, 168, 255, 0.45); } -.panel-success { - border-color: rgba(123, 228, 149, 0.42); - background: linear-gradient(180deg, rgba(123, 228, 149, 0.12), rgba(123, 228, 149, 0.06)); -} - -.auth-grid { - display: grid; - grid-template-columns: 1.3fr 1fr; - gap: 18px; - margin-top: 20px; -} -.auth-panel-title { - display: flex; - justify-content: space-between; - align-items: center; - gap: 12px; - margin-bottom: 8px; -} -.auth-kicker { - font-size: 12px; - letter-spacing: 0.08em; - text-transform: uppercase; - color: var(--muted); -} -.auth-divider { - height: 1px; - margin: 14px 0 10px; - border: 0; - background: var(--border); -} -.auth-list { - margin: 0 0 14px; - padding-left: 18px; - color: var(--muted); -} -.auth-list li + li { - margin-top: 6px; -} - -.notice-badge { - display: inline-flex; - align-items: center; - padding: 4px 10px; - border-radius: 999px; - border: 1px solid rgba(110, 168, 255, 0.55); - background: rgba(110, 168, 255, 0.18); - color: var(--text); - font-size: 12px; - font-weight: 600; -} - -.form { margin-top: 14px; display: grid; gap: 12px; } -.field { display: grid; gap: 6px; } -.label { color: var(--muted); font-size: 12px; } -.input { - width: 100%; - border-radius: 12px; - border: 1px solid var(--border); - background: rgba(255, 255, 255, 0.03); - color: var(--text); - padding: 10px 12px; - transition: border-color var(--transition-fast), background var(--transition-fast), outline-color var(--transition-fast); -} -.input:focus-visible { - outline: 2px solid var(--accent-strong); - outline-offset: 2px; -} -textarea.input { padding: 12px; resize: vertical; } -.actions { display: flex; flex-wrap: wrap; gap: 10px; margin-top: 4px; } - -.button { - display: inline-flex; - align-items: center; - justify-content: center; - gap: 8px; - height: 36px; - padding: 0 14px; - border-radius: 12px; - border: 1px solid rgba(110, 168, 255, 0.55); - background: rgba(110, 168, 255, 0.18); - color: var(--text); - font-weight: 600; - text-decoration: none; - cursor: pointer; - transition: all var(--transition-fast); -} -.button:hover { - background: rgba(110, 168, 255, 0.28); - border-color: rgba(143, 190, 255, 0.75); -} -.button-secondary { - border-color: var(--border); - background: rgba(255, 255, 255, 0.03); - color: var(--text); -} -.button-secondary:hover { background: rgba(255, 255, 255, 0.06); } - -.kv { - display: grid; - grid-template-columns: 120px 1fr; - gap: 10px; - margin-top: 10px; -} -.k { color: var(--muted); } -.v { overflow: auto; } - -.table { - width: 100%; - border-collapse: collapse; - margin-top: 10px; -} -.table th, .table td { - padding: 12px; - text-align: left; - border-bottom: 1px solid var(--border); - vertical-align: middle; -} -.table th { - font-size: 12px; - text-transform: uppercase; - letter-spacing: 0.08em; - color: var(--muted); -} - -.app-footer { - border-top: 1px solid var(--border); - padding: 20px 0; - color: var(--muted); -} -.footer-row { display: flex; justify-content: space-between; align-items: center; } - -@media (max-width: 700px) { - .container { width: min(1140px, calc(100% - 24px)); } - .page { padding-top: 24px; } - .page-title { align-items: flex-start; flex-direction: column; } - .auth-label { margin-right: 0; display: block; margin-bottom: 8px; } -} - -@media (max-width: 980px) { - .header-row { grid-template-columns: 1fr; align-items: start; } - .cards { grid-template-columns: 1fr; } - .auth-grid { grid-template-columns: 1fr; } -} From be76d354410c9dd3a6af8a9491f66c7ad02f6dca Mon Sep 17 00:00:00 2001 From: Linus Westling Date: Thu, 23 Apr 2026 09:48:04 +0200 Subject: [PATCH 4/7] Align DocumentServiceTest with S3 retry executor flow Run upload callbacks through the mocked retry executor and verify upload via executor invocation so patient upload tests match the current DocumentService behavior. Made-with: Cursor --- .../application/service/DocumentServiceTest.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/test/java/org/example/projektarendehantering/application/service/DocumentServiceTest.java b/src/test/java/org/example/projektarendehantering/application/service/DocumentServiceTest.java index f34272b..5324934 100644 --- a/src/test/java/org/example/projektarendehantering/application/service/DocumentServiceTest.java +++ b/src/test/java/org/example/projektarendehantering/application/service/DocumentServiceTest.java @@ -1,6 +1,5 @@ package org.example.projektarendehantering.application.service; -import io.awspring.cloud.s3.ObjectMetadata; import io.awspring.cloud.s3.S3Resource; import io.awspring.cloud.s3.S3Template; import org.example.projektarendehantering.common.AppException; @@ -20,10 +19,10 @@ import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.mock.web.MockMultipartFile; +import org.springframework.retry.RetryCallback; import org.springframework.test.util.ReflectionTestUtils; import java.io.IOException; -import java.io.InputStream; import java.time.Instant; import java.util.List; import java.util.Optional; @@ -66,6 +65,11 @@ class DocumentServiceTest { @BeforeEach void setUp() { ReflectionTestUtils.setField(documentService, "bucket", "test-bucket"); + lenient().when(s3RetryExecutor.execute(anyString(), any())).thenAnswer(invocation -> { + @SuppressWarnings("unchecked") + RetryCallback callback = invocation.getArgument(1); + return callback.doWithRetry(null); + }); UUID doctorId = UUID.randomUUID(); UUID managerId = UUID.randomUUID(); @@ -215,7 +219,7 @@ void uploadDocument_shouldAllowPatientOnOwnCase() throws IOException { DocumentDTO result = documentService.uploadDocument(patientActor, caseId, file); assertThat(result).isNotNull(); - verify(s3Template).upload(eq("test-bucket"), anyString(), any(InputStream.class), any(ObjectMetadata.class)); + verify(s3RetryExecutor).execute(eq("upload"), any()); } @Test From 1df33d37c946ab05b75c3acb279ed3001ece07c4 Mon Sep 17 00:00:00 2001 From: Linus Westling Date: Thu, 23 Apr 2026 10:26:17 +0200 Subject: [PATCH 5/7] Fixed the bugg that appeared in the demo --- .../application/service/EmployeeMapper.java | 10 +++++--- .../application/service/EmployeeService.java | 15 ++++++++--- .../common/GithubUsernameNormalizer.java | 20 +++++++++++++++ .../security/CustomOAuth2UserService.java | 3 ++- .../security/SecurityActorAdapter.java | 10 ++++++-- .../service/EmployeeMapperTest.java | 4 +-- .../service/EmployeeServiceTest.java | 23 ++++++++++++++++- .../security/CustomOAuth2UserServiceTest.java | 2 +- .../security/SecurityActorAdapterTest.java | 25 +++++++++++++++++++ 9 files changed, 98 insertions(+), 14 deletions(-) create mode 100644 src/main/java/org/example/projektarendehantering/common/GithubUsernameNormalizer.java diff --git a/src/main/java/org/example/projektarendehantering/application/service/EmployeeMapper.java b/src/main/java/org/example/projektarendehantering/application/service/EmployeeMapper.java index 5843328..08c9c79 100644 --- a/src/main/java/org/example/projektarendehantering/application/service/EmployeeMapper.java +++ b/src/main/java/org/example/projektarendehantering/application/service/EmployeeMapper.java @@ -5,6 +5,7 @@ import org.example.projektarendehantering.presentation.dto.EmployeeDTO; import org.example.projektarendehantering.presentation.dto.EmployeeUpdateDTO; import org.springframework.stereotype.Component; +import org.example.projektarendehantering.common.GithubUsernameNormalizer; import java.nio.charset.StandardCharsets; import java.util.UUID; @@ -25,15 +26,16 @@ public EmployeeDTO toDTO(EmployeeEntity entity) { public EmployeeEntity toEntity(EmployeeCreateDTO dto) { if (dto == null) return null; + String normalizedGithubUsername = GithubUsernameNormalizer.normalize(dto.getGithubUsername()); UUID id = null; - if (dto.getGithubUsername() != null && !dto.getGithubUsername().isBlank()) { - id = UUID.nameUUIDFromBytes(dto.getGithubUsername().getBytes(StandardCharsets.UTF_8)); + if (normalizedGithubUsername != null) { + id = UUID.nameUUIDFromBytes(normalizedGithubUsername.getBytes(StandardCharsets.UTF_8)); } return EmployeeEntity.builder() .id(id) .displayName(dto.getDisplayName()) - .githubUsername(dto.getGithubUsername()) + .githubUsername(normalizedGithubUsername) .role(dto.getRole()) .build(); } @@ -41,7 +43,7 @@ public EmployeeEntity toEntity(EmployeeCreateDTO dto) { public void updateEntity(EmployeeUpdateDTO dto, EmployeeEntity entity) { if (dto == null || entity == null) return; entity.setDisplayName(dto.getDisplayName()); - entity.setGithubUsername(dto.getGithubUsername()); + entity.setGithubUsername(GithubUsernameNormalizer.normalize(dto.getGithubUsername())); entity.setRole(dto.getRole()); } } diff --git a/src/main/java/org/example/projektarendehantering/application/service/EmployeeService.java b/src/main/java/org/example/projektarendehantering/application/service/EmployeeService.java index f48543a..2574a87 100644 --- a/src/main/java/org/example/projektarendehantering/application/service/EmployeeService.java +++ b/src/main/java/org/example/projektarendehantering/application/service/EmployeeService.java @@ -2,6 +2,7 @@ import org.example.projektarendehantering.common.Actor; import org.example.projektarendehantering.common.BadRequestException; +import org.example.projektarendehantering.common.GithubUsernameNormalizer; import org.example.projektarendehantering.common.NotAuthorizedException; import org.example.projektarendehantering.common.Role; import org.example.projektarendehantering.infrastructure.persistence.EmployeeEntity; @@ -29,9 +30,15 @@ public class EmployeeService { @Transactional public EmployeeDTO createEmployee(Actor actor, EmployeeCreateDTO dto) { requireCanManageEmployees(actor); + String normalizedGithubUsername = GithubUsernameNormalizer.normalize(dto.getGithubUsername()); + dto.setGithubUsername(normalizedGithubUsername); - if (employeeRepository.findByGithubUsername(dto.getGithubUsername()).isPresent()) { - throw new BadRequestException("EMPLOYEE_EXISTS", "Employee with username " + dto.getGithubUsername() + " already exists"); + if (normalizedGithubUsername == null) { + throw new BadRequestException("EMPLOYEE_USERNAME_REQUIRED", "Github username is required"); + } + + if (employeeRepository.findByGithubUsername(normalizedGithubUsername).isPresent()) { + throw new BadRequestException("EMPLOYEE_EXISTS", "Employee with username " + normalizedGithubUsername + " already exists"); } EmployeeEntity entity = employeeMapper.toEntity(dto); @@ -48,10 +55,12 @@ public EmployeeDTO updateEmployee(Actor actor, UUID id, EmployeeUpdateDTO dto) { EmployeeEntity entity = employeeRepository.findById(id) .orElseThrow(() -> new BadRequestException("EMPLOYEE_NOT_FOUND", "Employee not found")); - if (!entity.getGithubUsername().equals(dto.getGithubUsername())) { + String normalizedGithubUsername = GithubUsernameNormalizer.normalize(dto.getGithubUsername()); + if (!entity.getGithubUsername().equals(normalizedGithubUsername)) { throw new BadRequestException("EMPLOYEE_USERNAME_IMMUTABLE", "Github username cannot be changed"); } + dto.setGithubUsername(normalizedGithubUsername); employeeMapper.updateEntity(dto, entity); return employeeMapper.toDTO(employeeRepository.save(entity)); } diff --git a/src/main/java/org/example/projektarendehantering/common/GithubUsernameNormalizer.java b/src/main/java/org/example/projektarendehantering/common/GithubUsernameNormalizer.java new file mode 100644 index 0000000..437c94f --- /dev/null +++ b/src/main/java/org/example/projektarendehantering/common/GithubUsernameNormalizer.java @@ -0,0 +1,20 @@ +package org.example.projektarendehantering.common; + +import java.util.Locale; + +public final class GithubUsernameNormalizer { + + private GithubUsernameNormalizer() { + } + + public static String normalize(String githubUsername) { + if (githubUsername == null) { + return null; + } + String trimmed = githubUsername.trim(); + if (trimmed.isEmpty()) { + return null; + } + return trimmed.toLowerCase(Locale.ROOT); + } +} diff --git a/src/main/java/org/example/projektarendehantering/infrastructure/security/CustomOAuth2UserService.java b/src/main/java/org/example/projektarendehantering/infrastructure/security/CustomOAuth2UserService.java index 0ba6c60..b0a67ed 100644 --- a/src/main/java/org/example/projektarendehantering/infrastructure/security/CustomOAuth2UserService.java +++ b/src/main/java/org/example/projektarendehantering/infrastructure/security/CustomOAuth2UserService.java @@ -1,6 +1,7 @@ package org.example.projektarendehantering.infrastructure.security; import org.example.projektarendehantering.infrastructure.persistence.EmployeeRepository; +import org.example.projektarendehantering.common.GithubUsernameNormalizer; import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.authority.SimpleGrantedAuthority; import org.springframework.security.oauth2.client.userinfo.DefaultOAuth2UserService; @@ -26,7 +27,7 @@ public CustomOAuth2UserService(EmployeeRepository employeeRepository) { public OAuth2User loadUser(OAuth2UserRequest userRequest) throws OAuth2AuthenticationException { OAuth2User oAuth2User = loadBaseUser(userRequest); - String login = oAuth2User.getAttribute("login"); + String login = GithubUsernameNormalizer.normalize(oAuth2User.getAttribute("login")); Set authorities = new HashSet<>(oAuth2User.getAuthorities()); if (login != null) { diff --git a/src/main/java/org/example/projektarendehantering/infrastructure/security/SecurityActorAdapter.java b/src/main/java/org/example/projektarendehantering/infrastructure/security/SecurityActorAdapter.java index 377df4a..38a7d53 100644 --- a/src/main/java/org/example/projektarendehantering/infrastructure/security/SecurityActorAdapter.java +++ b/src/main/java/org/example/projektarendehantering/infrastructure/security/SecurityActorAdapter.java @@ -1,6 +1,7 @@ package org.example.projektarendehantering.infrastructure.security; import org.example.projektarendehantering.common.Actor; +import org.example.projektarendehantering.common.GithubUsernameNormalizer; import org.example.projektarendehantering.common.NotAuthorizedException; import org.example.projektarendehantering.common.Role; import org.example.projektarendehantering.infrastructure.persistence.EmployeeRepository; @@ -44,8 +45,13 @@ public Actor currentUser() { } } + String normalizedIdentity = GithubUsernameNormalizer.normalize(name); + if (normalizedIdentity == null) { + throw new NotAuthorizedException("Authenticated identity is missing"); + } + // Create a deterministic UUID based on the username/name - UUID userId = UUID.nameUUIDFromBytes(name.getBytes(StandardCharsets.UTF_8)); + UUID userId = UUID.nameUUIDFromBytes(normalizedIdentity.getBytes(StandardCharsets.UTF_8)); // 1. Try finding an employee with this UUID (OAuth/GitHub users) var employee = employeeRepository.findById(userId); @@ -73,6 +79,6 @@ public Actor currentUser() { role = Role.PATIENT; } - return new Actor(userId, role, null, name); + return new Actor(userId, role, null, normalizedIdentity); } } diff --git a/src/test/java/org/example/projektarendehantering/application/service/EmployeeMapperTest.java b/src/test/java/org/example/projektarendehantering/application/service/EmployeeMapperTest.java index c94afda..4089178 100644 --- a/src/test/java/org/example/projektarendehantering/application/service/EmployeeMapperTest.java +++ b/src/test/java/org/example/projektarendehantering/application/service/EmployeeMapperTest.java @@ -41,7 +41,7 @@ void toDTO_shouldMapAllFields() { void toEntity_shouldMapAllFields() { EmployeeCreateDTO dto = EmployeeCreateDTO.builder() .displayName("Bob") - .githubUsername("bob456") + .githubUsername(" Bob456 ") .role(Role.MANAGER) .build(); @@ -63,7 +63,7 @@ void updateEntity_shouldUpdateAllFields() { EmployeeUpdateDTO dto = EmployeeUpdateDTO.builder() .displayName("NewName") - .githubUsername("newuser") + .githubUsername(" NewUser ") .role(Role.MANAGER) .build(); diff --git a/src/test/java/org/example/projektarendehantering/application/service/EmployeeServiceTest.java b/src/test/java/org/example/projektarendehantering/application/service/EmployeeServiceTest.java index 09eca31..73d25b7 100644 --- a/src/test/java/org/example/projektarendehantering/application/service/EmployeeServiceTest.java +++ b/src/test/java/org/example/projektarendehantering/application/service/EmployeeServiceTest.java @@ -66,10 +66,11 @@ void getAllEmployees_shouldDenyDoctor() { @Test void createEmployee_shouldAllowManager() { - EmployeeCreateDTO dto = new EmployeeCreateDTO(); + EmployeeCreateDTO dto = new EmployeeCreateDTO("Name", " Gh_User ", Role.DOCTOR); EmployeeEntity entity = new EmployeeEntity(); EmployeeDTO resultDTO = new EmployeeDTO(UUID.randomUUID(), "Name", "gh_user", Role.DOCTOR, Instant.now()); + when(employeeRepository.findByGithubUsername("gh_user")).thenReturn(Optional.empty()); when(employeeMapper.toEntity(dto)).thenReturn(entity); when(employeeRepository.save(any())).thenReturn(entity); when(employeeMapper.toDTO(entity)).thenReturn(resultDTO); @@ -77,6 +78,7 @@ void createEmployee_shouldAllowManager() { EmployeeDTO result = employeeService.createEmployee(managerActor, dto); assertThat(result).isNotNull(); + assertThat(dto.getGithubUsername()).isEqualTo("gh_user"); verify(employeeRepository).save(any()); } @@ -119,6 +121,25 @@ void updateEmployee_shouldThrowIfUsernameChanged() { .hasMessageContaining("Github username cannot be changed"); } + @Test + void updateEmployee_shouldAllowEquivalentUsernameAfterNormalization() { + UUID id = UUID.randomUUID(); + EmployeeUpdateDTO dto = new EmployeeUpdateDTO("New Name", " GH_USER ", Role.DOCTOR); + EmployeeEntity entity = new EmployeeEntity(); + entity.setGithubUsername("gh_user"); + EmployeeDTO resultDTO = new EmployeeDTO(id, "New Name", "gh_user", Role.DOCTOR, Instant.now()); + + when(employeeRepository.findById(id)).thenReturn(Optional.of(entity)); + when(employeeRepository.save(entity)).thenReturn(entity); + when(employeeMapper.toDTO(entity)).thenReturn(resultDTO); + + EmployeeDTO result = employeeService.updateEmployee(managerActor, id, dto); + + assertThat(result).isNotNull(); + assertThat(dto.getGithubUsername()).isEqualTo("gh_user"); + verify(employeeMapper).updateEntity(dto, entity); + } + @Test void deleteEmployee_shouldDeleteIfManager() { UUID id = UUID.randomUUID(); diff --git a/src/test/java/org/example/projektarendehantering/infrastructure/security/CustomOAuth2UserServiceTest.java b/src/test/java/org/example/projektarendehantering/infrastructure/security/CustomOAuth2UserServiceTest.java index 2591e4f..499bb1b 100644 --- a/src/test/java/org/example/projektarendehantering/infrastructure/security/CustomOAuth2UserServiceTest.java +++ b/src/test/java/org/example/projektarendehantering/infrastructure/security/CustomOAuth2UserServiceTest.java @@ -48,7 +48,7 @@ protected OAuth2User loadBaseUser(OAuth2UserRequest userRequest) { private OAuth2User mockOAuth2User() { return new DefaultOAuth2User( Collections.emptyList(), - Map.of("login", "testuser", "id", 123), + Map.of("login", " TestUser ", "id", 123), "id" ); } diff --git a/src/test/java/org/example/projektarendehantering/infrastructure/security/SecurityActorAdapterTest.java b/src/test/java/org/example/projektarendehantering/infrastructure/security/SecurityActorAdapterTest.java index 2f23eca..03479f4 100644 --- a/src/test/java/org/example/projektarendehantering/infrastructure/security/SecurityActorAdapterTest.java +++ b/src/test/java/org/example/projektarendehantering/infrastructure/security/SecurityActorAdapterTest.java @@ -121,6 +121,31 @@ void currentUser_whenOAuth2Authentication_shouldUseLoginAttribute() { assertThat(actor.role()).isEqualTo(Role.PENDING); } + @Test + void currentUser_whenOAuth2LoginHasMixedCase_shouldNormalizeBeforeIdLookup() { + String login = " TestUser "; + String normalizedLogin = "testuser"; + UUID userId = UUID.nameUUIDFromBytes(normalizedLogin.getBytes(StandardCharsets.UTF_8)); + + OAuth2AuthenticationToken oauth2Token = mock(OAuth2AuthenticationToken.class); + OAuth2User oauth2User = mock(OAuth2User.class); + + when(securityContext.getAuthentication()).thenReturn(oauth2Token); + when(oauth2Token.isAuthenticated()).thenReturn(true); + when(oauth2Token.getName()).thenReturn("unused-name"); + when(oauth2Token.getPrincipal()).thenReturn(oauth2User); + when(oauth2User.getAttribute("login")).thenReturn(login); + + when(employeeRepository.findById(userId)).thenReturn(Optional.empty()); + when(userAccountRepository.findByEmail("unused-name")).thenReturn(Optional.empty()); + doReturn(Collections.emptyList()).when(oauth2Token).getAuthorities(); + + Actor actor = securityActorAdapter.currentUser(); + + assertThat(actor.userId()).isEqualTo(userId); + assertThat(actor.githubUsername()).isEqualTo(normalizedLogin); + } + @Test void currentUser_whenEmployeeNotFound_shouldFallbackToAuthorities_Manager() { String username = "manager-user"; From 73b951ec4b2fca86b0ceb58c322643874d388304 Mon Sep 17 00:00:00 2001 From: Linus Westling Date: Thu, 23 Apr 2026 11:11:42 +0200 Subject: [PATCH 6/7] fixed app.css --- .../application/service/EmployeeService.java | 3 +- .../service/FailedS3DeletionService.java | 16 +- .../application/service/S3RetryExecutor.java | 57 ++-- .../config/AuditWebMvcConfig.java | 1 + .../infrastructure/config/SecurityConfig.java | 2 +- src/main/resources/static/app.css | 270 ++++++++++++++++ .../resources/templates/fragments/head.html | 290 +----------------- 7 files changed, 314 insertions(+), 325 deletions(-) create mode 100644 src/main/resources/static/app.css diff --git a/src/main/java/org/example/projektarendehantering/application/service/EmployeeService.java b/src/main/java/org/example/projektarendehantering/application/service/EmployeeService.java index 2574a87..8a1cdb0 100644 --- a/src/main/java/org/example/projektarendehantering/application/service/EmployeeService.java +++ b/src/main/java/org/example/projektarendehantering/application/service/EmployeeService.java @@ -16,6 +16,7 @@ import java.time.Instant; import java.util.List; +import java.util.Objects; import java.util.Optional; import java.util.UUID; import java.util.stream.Collectors; @@ -56,7 +57,7 @@ public EmployeeDTO updateEmployee(Actor actor, UUID id, EmployeeUpdateDTO dto) { .orElseThrow(() -> new BadRequestException("EMPLOYEE_NOT_FOUND", "Employee not found")); String normalizedGithubUsername = GithubUsernameNormalizer.normalize(dto.getGithubUsername()); - if (!entity.getGithubUsername().equals(normalizedGithubUsername)) { + if (!Objects.equals(entity.getGithubUsername(), normalizedGithubUsername)) { throw new BadRequestException("EMPLOYEE_USERNAME_IMMUTABLE", "Github username cannot be changed"); } diff --git a/src/main/java/org/example/projektarendehantering/application/service/FailedS3DeletionService.java b/src/main/java/org/example/projektarendehantering/application/service/FailedS3DeletionService.java index d6d6aa9..031709b 100644 --- a/src/main/java/org/example/projektarendehantering/application/service/FailedS3DeletionService.java +++ b/src/main/java/org/example/projektarendehantering/application/service/FailedS3DeletionService.java @@ -58,7 +58,7 @@ public void enqueue(String bucket, String s3Key, Exception e) { .build(); failedS3DeletionRepository.save(entity); log.warn("Queued failed S3 deletion for retry. bucket={}, key={}", bucket, s3Key); - auditService.record(AuditEventEntity.builder() + recordAuditBestEffort(AuditEventEntity.builder() .eventName("DOCUMENT_S3_DELETE_QUEUED") .description("Queued failed S3 deletion for retry") .queryString("bucket=" + bucket + "&s3Key=" + s3Key + "&error=" + trimmed) @@ -97,7 +97,7 @@ public void processOne(UUID id) { }); failedS3DeletionRepository.delete(item); log.info("Recovered failed S3 deletion. bucket={}, key={}", item.getBucket(), item.getS3Key()); - auditService.record(AuditEventEntity.builder() + recordAuditBestEffort(AuditEventEntity.builder() .eventName("DOCUMENT_S3_DELETE_RECOVERED") .description("Recovered failed S3 deletion from retry queue") .queryString("bucket=" + item.getBucket() + "&s3Key=" + item.getS3Key() + "&attempts=" + item.getAttemptCount()) @@ -121,7 +121,7 @@ public void processOne(UUID id) { failedS3DeletionRepository.save(item); log.warn("Failed retrying S3 deletion. bucket={}, key={}, attempts={}", item.getBucket(), item.getS3Key(), item.getAttemptCount(), ex); - auditService.record(AuditEventEntity.builder() + recordAuditBestEffort(AuditEventEntity.builder() .eventName("DOCUMENT_S3_DELETE_RETRY_FAILED") .description("Retry failed for queued S3 deletion") .queryString("bucket=" + item.getBucket() + "&s3Key=" + item.getS3Key() @@ -139,7 +139,7 @@ private long computeRetryDelaySeconds(int attemptCount) { private void markDeadLetter(FailedS3DeletionEntity item, String reason) { failedS3DeletionRepository.delete(item); - auditService.record(AuditEventEntity.builder() + recordAuditBestEffort(AuditEventEntity.builder() .eventName("DOCUMENT_S3_DELETE_DEAD_LETTERED") .description("Queued S3 deletion reached max retry attempts and was dead-lettered") .queryString("bucket=" + item.getBucket() + "&s3Key=" + item.getS3Key() @@ -148,6 +148,14 @@ private void markDeadLetter(FailedS3DeletionEntity item, String reason) { .build()); } + private void recordAuditBestEffort(AuditEventEntity event) { + try { + auditService.record(event); + } catch (Exception ex) { + log.warn("Failed to record S3 deletion audit event. eventName={}", event.getEventName(), ex); + } + } + private String trimError(Exception e) { String message = e.getMessage(); if (message == null) { diff --git a/src/main/java/org/example/projektarendehantering/application/service/S3RetryExecutor.java b/src/main/java/org/example/projektarendehantering/application/service/S3RetryExecutor.java index e70f2c0..fd23dd2 100644 --- a/src/main/java/org/example/projektarendehantering/application/service/S3RetryExecutor.java +++ b/src/main/java/org/example/projektarendehantering/application/service/S3RetryExecutor.java @@ -58,28 +58,33 @@ public T execute(String operationName, RetryCallback ca } public boolean isRetryable(Throwable throwable) { - Throwable root = rootCause(throwable); - - if (root instanceof S3Exception s3Exception) { - return isRetryableStatusOrAwsError( - s3Exception.statusCode(), - s3Exception.awsErrorDetails() != null ? s3Exception.awsErrorDetails().errorCode() : null - ); - } - if (root instanceof AwsServiceException awsServiceException) { - return isRetryableStatusOrAwsError( - awsServiceException.statusCode(), - awsServiceException.awsErrorDetails() != null ? awsServiceException.awsErrorDetails().errorCode() : null - ); - } - if (root instanceof NonRetryableException) { - return false; - } - if (root instanceof RetryableException) { - return true; - } - if (root instanceof SdkClientException sdkClientException) { - return sdkClientException.retryable() || isLikelyTransientClientError(sdkClientException.getMessage()); + Throwable current = throwable; + while (current != null) { + if (current instanceof S3Exception s3Exception) { + return isRetryableStatusOrAwsError( + s3Exception.statusCode(), + s3Exception.awsErrorDetails() != null ? s3Exception.awsErrorDetails().errorCode() : null + ); + } + if (current instanceof AwsServiceException awsServiceException) { + return isRetryableStatusOrAwsError( + awsServiceException.statusCode(), + awsServiceException.awsErrorDetails() != null ? awsServiceException.awsErrorDetails().errorCode() : null + ); + } + if (current instanceof NonRetryableException) { + return false; + } + if (current instanceof RetryableException) { + return true; + } + if (current instanceof SdkClientException sdkClientException) { + return sdkClientException.retryable() || isLikelyTransientClientError(sdkClientException.getMessage()); + } + if (current.getCause() == current) { + break; + } + current = current.getCause(); } return false; } @@ -132,14 +137,6 @@ private boolean isRetryableStatusOrAwsError(int statusCode, String awsErrorCode) return RETRYABLE_HTTP_STATUS.contains(statusCode) || isRetryableAwsErrorCode(awsErrorCode); } - private Throwable rootCause(Throwable throwable) { - Throwable current = throwable; - while (current.getCause() != null && current.getCause() != current) { - current = current.getCause(); - } - return current; - } - private boolean isLikelyTransientClientError(String message) { if (message == null || message.isBlank()) { return false; diff --git a/src/main/java/org/example/projektarendehantering/infrastructure/config/AuditWebMvcConfig.java b/src/main/java/org/example/projektarendehantering/infrastructure/config/AuditWebMvcConfig.java index 981a67d..7f543ac 100644 --- a/src/main/java/org/example/projektarendehantering/infrastructure/config/AuditWebMvcConfig.java +++ b/src/main/java/org/example/projektarendehantering/infrastructure/config/AuditWebMvcConfig.java @@ -20,6 +20,7 @@ public void addInterceptors(InterceptorRegistry registry) { .addPathPatterns("/ui/**", "/api/**") .excludePathPatterns( "/static/**", + "/app.css", "/app.js", "/error**", "/login**" diff --git a/src/main/java/org/example/projektarendehantering/infrastructure/config/SecurityConfig.java b/src/main/java/org/example/projektarendehantering/infrastructure/config/SecurityConfig.java index 7049f23..cf17a12 100644 --- a/src/main/java/org/example/projektarendehantering/infrastructure/config/SecurityConfig.java +++ b/src/main/java/org/example/projektarendehantering/infrastructure/config/SecurityConfig.java @@ -34,7 +34,7 @@ public SecurityConfig(LocalUserDetailsService localUserDetailsService) { public SecurityFilterChain securityFilterChain(HttpSecurity http, CustomOAuth2UserService customOAuth2UserService) throws Exception { http .authorizeHttpRequests(authorize -> authorize - .requestMatchers("/login**", "/register", "/error**", "/static/**", "/app.js", "/webjars/**").permitAll() + .requestMatchers("/login**", "/register", "/error**", "/static/**", "/app.css", "/app.js", "/webjars/**").permitAll() .anyRequest().authenticated() ) .oauth2Login(oauth2 -> oauth2 diff --git a/src/main/resources/static/app.css b/src/main/resources/static/app.css new file mode 100644 index 0000000..28dd506 --- /dev/null +++ b/src/main/resources/static/app.css @@ -0,0 +1,270 @@ +:root { + --bg: #0b1020; + --panel: #101a33; + --panel2: #0f1730; + --text: #e7ecff; + --muted: rgba(231, 236, 255, 0.7); + --border: rgba(231, 236, 255, 0.12); + --accent: #6ea8ff; + --accent-strong: #8fbeff; + --success: #7be495; + --danger: #ff6e8a; + --shadow: 0 10px 30px rgba(0, 0, 0, 0.35); + --radius: 16px; + --transition-fast: 160ms ease; +} + +* { box-sizing: border-box; } +html, body { height: 100%; } +body { + margin: 0; + font-family: ui-sans-serif, system-ui, -apple-system, Segoe UI, Roboto, Arial, "Apple Color Emoji", "Segoe UI Emoji"; + background: radial-gradient(1200px 700px at 20% 0%, rgba(110, 168, 255, 0.25), transparent 60%), + radial-gradient(900px 500px at 70% 20%, rgba(142, 97, 255, 0.18), transparent 60%), + var(--bg); + color: var(--text); + line-height: 1.5; +} + +code, pre { font-family: ui-monospace, SFMono-Regular, Menlo, Monaco, Consolas, "Liberation Mono", "Courier New", monospace; } +code { color: rgba(231, 236, 255, 0.95); } +a { color: inherit; } + +.container { + width: min(1140px, calc(100% - 40px)); + margin: 0 auto; +} + +.app-header { + position: sticky; + top: 0; + z-index: 10; + background: rgba(11, 16, 32, 0.75); + backdrop-filter: blur(10px); + border-bottom: 1px solid var(--border); +} + +.header-row { + display: grid; + grid-template-columns: 1fr auto auto; + gap: 20px; + align-items: center; + padding: 14px 0 12px; +} + +.brand-link { + color: var(--text); + text-decoration: none; + font-weight: 700; + letter-spacing: 0.25px; + font-size: 18px; +} + +.nav { display: flex; gap: 8px; flex-wrap: wrap; } +.nav-link { + color: var(--muted); + text-decoration: none; + padding: 8px 10px; + border-radius: 10px; + border: 1px solid transparent; + transition: all var(--transition-fast); +} +.nav-link:hover { + color: var(--text); + border-color: var(--border); + background: rgba(255, 255, 255, 0.03); +} + +.auth { + display: flex; + gap: 10px; + align-items: center; +} +.auth-label { font-size: 14px; color: var(--muted); margin-right: 10px; } + +.page { padding: 30px 0 46px; } +.page-title { display: flex; justify-content: space-between; align-items: center; gap: 12px; } +h1 { margin: 0 0 12px; font-size: clamp(28px, 4vw, 34px); line-height: 1.18; letter-spacing: 0.1px; } +h2 { margin: 0 0 10px; font-size: 20px; line-height: 1.3; } +h3 { margin: 0 0 8px; font-size: 16px; line-height: 1.35; } +h4 { margin: 0 0 8px; font-size: 16px; line-height: 1.35; } +p { margin: 0 0 14px; color: var(--muted); max-width: 75ch; } +.muted { color: var(--muted); } + +.cards { + display: grid; + grid-template-columns: repeat(auto-fit, minmax(240px, 1fr)); + gap: 16px; + margin-top: 20px; +} +.card { + display: block; + text-decoration: none; + color: var(--text); + background: linear-gradient(180deg, rgba(255,255,255,0.04), rgba(255,255,255,0.02)); + border: 1px solid var(--border); + border-radius: var(--radius); + padding: 18px; + box-shadow: var(--shadow); + transition: transform var(--transition-fast), border-color var(--transition-fast), background var(--transition-fast); +} +.card:hover { + border-color: rgba(110, 168, 255, 0.45); + background: linear-gradient(180deg, rgba(255,255,255,0.06), rgba(255,255,255,0.03)); + transform: translateY(-1px); +} +.card p { margin: 6px 0 0; } + +.panel { + background: linear-gradient(180deg, rgba(255,255,255,0.04), rgba(255,255,255,0.02)); + border: 1px solid var(--border); + border-radius: var(--radius); + padding: 18px; + box-shadow: var(--shadow); +} +.panel + .panel { margin-top: 14px; } +.panel-error { border-color: rgba(255, 110, 138, 0.35); } +.panel-notice { border-color: rgba(110, 168, 255, 0.45); } +.panel-success { + border-color: rgba(123, 228, 149, 0.42); + background: linear-gradient(180deg, rgba(123, 228, 149, 0.12), rgba(123, 228, 149, 0.06)); +} + +.auth-grid { + display: grid; + grid-template-columns: 1.3fr 1fr; + gap: 18px; + margin-top: 20px; +} +.auth-panel-title { + display: flex; + justify-content: space-between; + align-items: center; + gap: 12px; + margin-bottom: 8px; +} +.auth-kicker { + font-size: 12px; + letter-spacing: 0.08em; + text-transform: uppercase; + color: var(--muted); +} +.auth-divider { + height: 1px; + margin: 14px 0 10px; + border: 0; + background: var(--border); +} +.auth-list { + margin: 0 0 14px; + padding-left: 18px; + color: var(--muted); +} +.auth-list li + li { + margin-top: 6px; +} + +.notice-badge { + display: inline-flex; + align-items: center; + padding: 4px 10px; + border-radius: 999px; + border: 1px solid rgba(110, 168, 255, 0.55); + background: rgba(110, 168, 255, 0.18); + color: var(--text); + font-size: 12px; + font-weight: 600; +} + +.form { margin-top: 14px; display: grid; gap: 12px; } +.field { display: grid; gap: 6px; } +.label { color: var(--muted); font-size: 12px; } +.input { + width: 100%; + border-radius: 12px; + border: 1px solid var(--border); + background: rgba(255, 255, 255, 0.03); + color: var(--text); + padding: 10px 12px; + transition: border-color var(--transition-fast), background var(--transition-fast), outline-color var(--transition-fast); +} +.input:focus-visible { + outline: 2px solid var(--accent-strong); + outline-offset: 2px; +} +textarea.input { padding: 12px; resize: vertical; } +.actions { display: flex; flex-wrap: wrap; gap: 10px; margin-top: 4px; } + +.button { + display: inline-flex; + align-items: center; + justify-content: center; + gap: 8px; + height: 36px; + padding: 0 14px; + border-radius: 12px; + border: 1px solid rgba(110, 168, 255, 0.55); + background: rgba(110, 168, 255, 0.18); + color: var(--text); + font-weight: 600; + text-decoration: none; + cursor: pointer; + transition: all var(--transition-fast); +} +.button:hover { + background: rgba(110, 168, 255, 0.28); + border-color: rgba(143, 190, 255, 0.75); +} +.button-secondary { + border-color: var(--border); + background: rgba(255, 255, 255, 0.03); + color: var(--text); +} +.button-secondary:hover { background: rgba(255, 255, 255, 0.06); } + +.kv { + display: grid; + grid-template-columns: 120px 1fr; + gap: 10px; + margin-top: 10px; +} +.k { color: var(--muted); } +.v { overflow: auto; } + +.table { + width: 100%; + border-collapse: collapse; + margin-top: 10px; +} +.table th, .table td { + padding: 12px; + text-align: left; + border-bottom: 1px solid var(--border); + vertical-align: middle; +} +.table th { + font-size: 12px; + text-transform: uppercase; + letter-spacing: 0.08em; + color: var(--muted); +} + +.app-footer { + border-top: 1px solid var(--border); + padding: 20px 0; + color: var(--muted); +} +.footer-row { display: flex; justify-content: space-between; align-items: center; } + +@media (max-width: 700px) { + .container { width: min(1140px, calc(100% - 24px)); } + .page { padding-top: 24px; } + .page-title { align-items: flex-start; flex-direction: column; } + .auth-label { margin-right: 0; display: block; margin-bottom: 8px; } +} + +@media (max-width: 980px) { + .header-row { grid-template-columns: 1fr; align-items: start; } + .cards { grid-template-columns: 1fr; } + .auth-grid { grid-template-columns: 1fr; } +} diff --git a/src/main/resources/templates/fragments/head.html b/src/main/resources/templates/fragments/head.html index 50865c8..a5f1140 100644 --- a/src/main/resources/templates/fragments/head.html +++ b/src/main/resources/templates/fragments/head.html @@ -4,294 +4,6 @@ Ärendehantering - + From df180303b0b61c081bd7c791de2246ac31a0956e Mon Sep 17 00:00:00 2001 From: mattknatt Date: Thu, 23 Apr 2026 11:54:43 +0200 Subject: [PATCH 7/7] Remove redundant exception handling in S3 upload logic and update related test case --- .../application/service/DocumentService.java | 6 +----- .../application/service/DocumentServiceTest.java | 3 ++- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/example/projektarendehantering/application/service/DocumentService.java b/src/main/java/org/example/projektarendehantering/application/service/DocumentService.java index 15f734e..d82506b 100644 --- a/src/main/java/org/example/projektarendehantering/application/service/DocumentService.java +++ b/src/main/java/org/example/projektarendehantering/application/service/DocumentService.java @@ -98,11 +98,7 @@ public DocumentDTO uploadDocument(Actor actor, UUID caseId, MultipartFile file) .contentType(contentType) .build(); s3RetryExecutor.execute("upload", context -> { - try { - s3Template.upload(bucket, s3Key, new ByteArrayInputStream(fileBytes), metadata); - } catch (IOException ioException) { - throw new AppException("S3_UPLOAD_STREAM_FAILED", "Failed to read upload input stream", ioException); - } + s3Template.upload(bucket, s3Key, new ByteArrayInputStream(fileBytes), metadata); return null; }); diff --git a/src/test/java/org/example/projektarendehantering/application/service/DocumentServiceTest.java b/src/test/java/org/example/projektarendehantering/application/service/DocumentServiceTest.java index e5b5f3b..8216480 100644 --- a/src/test/java/org/example/projektarendehantering/application/service/DocumentServiceTest.java +++ b/src/test/java/org/example/projektarendehantering/application/service/DocumentServiceTest.java @@ -1,5 +1,6 @@ package org.example.projektarendehantering.application.service; +import io.awspring.cloud.s3.ObjectMetadata; import io.awspring.cloud.s3.S3Resource; import io.awspring.cloud.s3.S3Template; import org.example.projektarendehantering.common.AppException; @@ -180,7 +181,7 @@ void deleteDocument_shouldAllowOwner() { @Test void uploadDocument_shouldBubbleDegradedCodeWhenS3TransientlyUnavailable() { - MockMultipartFile file = new MockMultipartFile("file", "test.txt", "text/plain", "hello".getBytes()); + MockMultipartFile file = new MockMultipartFile("file", "test.pdf", "application/pdf", "%PDF-1.4 test".getBytes()); when(caseRepository.findById(caseId)).thenReturn(Optional.of(caseEntity)); when(s3RetryExecutor.execute(eq("upload"), any())) .thenThrow(new AppException("S3_SERVICE_DEGRADED", "Temporary S3 issue while trying to upload"));