diff --git a/backend-api/docs/NOTIFICATION_SERVICE.md b/backend-api/docs/NOTIFICATION_SERVICE.md new file mode 100644 index 0000000..070ce46 --- /dev/null +++ b/backend-api/docs/NOTIFICATION_SERVICE.md @@ -0,0 +1,213 @@ +# Notification Service: Persisted In-App Notifications & Email Fan-Out + +Implements the Notification Service issue: appointment lifecycle changes +(booked, accepted, declined, updated, cancelled, deleted) now produce a +persisted, readable in-app notification for each affected party, plus a +best-effort transactional email — instead of silently changing state that +nobody is told about. + +## What was there before + +`NotificationServiceImpl` was an empty `@Service` implementing an empty +`NotificationService` interface. `Notification` (the JPA entity) and +`NotificationRepository` existed but were never referenced from anywhere else +in the codebase. `MailServiceImp` could already send transactional email +through Brevo, but nothing called it from the appointment flow. None of the +three pieces were connected, and no appointment lifecycle method — booking, +accepting, updating, cancelling, or deleting — told either party anything had +changed. + +## New dependencies + +None. Everything here — Spring's `@Async`/`@EnableAsync`, `TransactionSynchronizationManager`, +Spring Data JPA paging, springdoc — was already on the classpath. + +## Architectural decisions + +### 1. Email fan-out runs off the request thread, after the transaction commits + +The issue requires that a slow or down mail provider never fail or block the +appointment operation, and that a failed send never leave a booking +half-committed. Two mechanisms together guarantee this: + +- **A dedicated executor** (`AsyncConfig` / `NotificationEmailDispatcher`). + `NotificationEmailDispatcher.dispatch` is `@Async` on its own bean — + `@Async` only intercepts calls that arrive from outside the declaring bean, + so it has to live on a class of its own rather than as a method on + `NotificationServiceImpl` that method would call on itself. +- **Deferred until commit.** `NotificationServiceImpl#scheduleEmail` checks + `TransactionSynchronizationManager.isSynchronizationActive()`. If the + appointment write that produced this notification is still inside a + transaction, the email is scheduled via `registerSynchronization(...).afterCommit()` + rather than sent immediately — so a notification is never emailed for a + change that ends up rolled back, and the email call is never made on the + thread holding the database connection. + +A failed or thrown-exception send is caught inside `dispatch` and recorded on +the notification's `emailStatus` (`PENDING`/`SENT`/`FAILED`); it is never +rethrown; there is nothing left to observe it by the time it runs, and +resurfacing it would have nowhere useful to go. + +### 2. The in-app notification is not optional or best-effort + +Unlike the email, persisting the `Notification` row happens synchronously, in +the same transaction as the appointment write. If the appointment save rolls +back, the notification never existed either; if it commits, the notification +is guaranteed to exist before the (still-pending) email is even scheduled. + +### 3. Ownership is scoped by email, not by a linked user id + +`Client`, `SkilledWorker`, and `UserAccount` (the JWT login identity) are +deliberately separate tables in this codebase with no foreign key between +them (see `UserAccount`'s class Javadoc) — email is the only identifier they +share, and it is what `JwtAuthenticationFilter` puts in the security +principal. `Notification.recipientEmail` is scoped the same way: +`NotificationController` reads the caller's email via +`@AuthenticationPrincipal String`, and every repository query is filtered by +it — `GET /api/v1/notifications`, `GET /api/v1/notifications/unread-count`, +`PUT /api/v1/notifications/{id}/read`, and `PUT /api/v1/notifications/read-all` +all only ever see or touch the caller's own rows. A notification id that +exists but belongs to someone else 404s exactly like one that doesn't exist, +so a caller can't distinguish "not mine" from "never existed". + +### 4. A pre-existing gap this depended on: booking never resolved a `Client` + +`AppointmentServiceImpl#bookAppointment` mapped `BookAppointmentRequest` to +`Appointment` via `ModelMapper` and explicitly resolved the `SkilledWorker` by +id (with a comment explaining why: ModelMapper can silently build a transient +entity from just an id, which either fails to persist or corrupts the row) — +but it never did the same for the `Client`. Booking notifications need a real +`client.getEmail()`/`getFullName()`, so `bookAppointment` now resolves the +`Client` from `clientId` the same way it already did for the worker. +`ClientServiceImpl#bookAppointment` still re-sets and re-saves the client +afterwards; that's now redundant but harmless, and left alone to minimize the +blast radius of this change. + +### 5. Two appointment-creation paths, one notification call + +Appointments are created in two places: the legacy +`AppointmentServiceImpl#bookAppointment` (`POST /api/v1/client/bookAppointment`) +and the concurrency-safe two-step `SlotReservationService#confirm` +(`POST /api/v1/booking/reservations/{id}/confirm`, see `APPOINTMENT_BOOKING.md`). +Both now call `NotificationService#notifyAppointmentEvent(appointment, +APPOINTMENT_BOOKED)` at their respective points of success, so a booking +produces a notification regardless of which flow made it. + +## Schema / migrations + +No migration file, and none is needed: this codebase has no Flyway/Liquibase +and manages schema via `spring.jpa.hibernate.ddl-auto=update` +(`application.properties`). `Notification` already existed as an unused +table, so this PR is additive on top of it rather than a fresh table: + +- Every new column (`recipient_email`, `recipient_role`, `type`, `title`, + `message`, `appointment_id`, `is_read`, `read_at`, `email_status`, + `created_at`) is new — `update` mode creates them on first deploy. +- The identity column deliberately **keeps its original name** + (`notification_id`, from the pre-existing `notificationId` field) instead + of being renamed to `id` to match newer entities in this codebase. + `ddl-auto=update` only ever adds missing columns; it does not rename or + drop them, so renaming the primary key column could leave two identity + columns behind on an already-deployed database. The old, now-unused + `user_id`/`status` columns are left in place for the same reason — inert, + but not worth an ALTER TABLE this schema-management strategy can't express + safely. See `docs/ESCROW_ORCHESTRATION.md`/`docs/APPOINTMENT_BOOKING.md` for + the same tradeoff made elsewhere in this codebase. + +## Endpoints + +All under `/api/v1/notifications`, authenticated (JWT bearer), scoped to the +caller: + +| Method & path | Purpose | +|-----------------------------------------|---------------------------------------------| +| `GET /` | Paginated list, newest first | +| `GET /unread-count` | Count of unread notifications | +| `PUT /{id}/read` | Mark one notification read (404 if not the caller's) | +| `PUT /read-all` | Mark all of the caller's unread notifications read | + +Documented via springdoc (`@Operation`); errors follow the same RFC 7807 +(`application/problem+json`) contract as the rest of the API — a new +`NotificationNotFoundException` → 404 entry was added to +`GlobalExceptionHandler`. + +## Pagination + +`GET /api/v1/notifications` defaults to a page size of 20 +(`@PageableDefault` on `NotificationController#list`) and is now hard-capped +at 100 regardless of what a caller passes as `?size=` +(`spring.data.web.pageable.max-page-size`, `application.properties`) — Spring +Data's own default cap (2000) was too high to call safe for a public list +endpoint. Sort defaults to `createdAt DESC`, matching the +`idx_notifications_recipient_created` index below, so the common case never +falls back to a filesort. + +## Indexing & retention + +`Notification` carries two indexes matching its two access patterns: +`idx_notifications_recipient_created` (`recipient_email, created_at`) for the +paginated list, and `idx_notifications_recipient_unread` +(`recipient_email, is_read`) for the unread count and `markAllAsRead`'s +lookup. + +There is no retention/archival job. This table now gets a row per recipient +per lifecycle event, so it grows unboundedly with appointment volume — that's +an acceptable starting point for this issue's scope, but worth flagging as +follow-up work before volume makes it a problem: either a scheduled sweep +that deletes/archives read notifications past some age (mirroring +`SlotReservationService#expireLapsedHolds`'s `@Scheduled` pattern), or a +TTL-based partitioning strategy if this grows enough to matter operationally. + +## Observability + +A failed email send is recorded two ways: the notification's own +`emailStatus=FAILED` (queryable directly), and an `ERROR`-level log line from +`NotificationEmailDispatcher` with the notification id, recipient, and +exception. There is no emitted metric/counter — this codebase has no +Micrometer/Actuator dependency today, and adding one is a cross-cutting, +new-dependency decision that belongs in its own PR rather than folding into +this feature. Log-based alerting on `NotificationEmailDispatcher` at `ERROR` +is the interim path to ops visibility. + +## Concurrency + +`Notification` intentionally has no `@Version`/optimistic-locking column. The +only post-insert mutations are `read` flipping `false → true` and +`emailStatus` being set exactly once by the dispatcher; both are idempotent +and monotonic, and every read/write is already scoped by `recipientEmail` +first. Two concurrent mark-read calls on the same row converge on the same +end state, so optimistic locking would only risk throwing on a harmless race, +not prevent a lost update. + +## Testing + +- `NotificationServiceImplTest` — recipient selection per lifecycle event + (e.g. accepted/declined only notify the client), content, pagination + delegation, and that mark-read/mark-all-read are scoped to the caller's + email. +- `NotificationEmailDispatcherTest` — the mail-provider-down case: a thrown + exception from `MailService` is caught, marks the notification `FAILED`, + and never propagates. +- `AppointmentServiceImplTest` — each lifecycle method + (book/cancel/update/accept/delete) calls `notifyAppointmentEvent` with the + correct `NotificationType`. +- `NotificationControllerTest` — HTTP-level ownership scoping (list, + unread-count, mark-read, mark-all-read) against a real JWT and a real + database. Cross-user access returns `404`, not `403`: recipient scoping is + by `recipientEmail`, not by an id the caller passes, so there's no + "wrong owner, right id" case to distinguish from "no such notification" — + see architectural decision #3. +- `NotificationEmailFanOutIntegrationTest` — end-to-end proof (real + `AppointmentService`, real transaction, real `@Async` executor, mocked + `MailService` throwing) that a down mail provider still lets + `bookAppointment` commit, and that the resulting notifications land as + `emailStatus=FAILED` rather than blocking or rolling back the booking. +- Existing booking tests (`SlotReservationIntegrationTest`, + `BookingControllerTest`, `ClientServiceTest`) now mock `MailService` so + they don't make real calls to the mail provider now that booking triggers + a notification fan-out. + +## CI + +`test.yml` already caches Maven dependencies (`cache: maven` on +`actions/setup-java@v4`) — no change needed there. diff --git a/backend-api/pom.xml b/backend-api/pom.xml index 59a6fb0..bb73a0e 100644 --- a/backend-api/pom.xml +++ b/backend-api/pom.xml @@ -223,6 +223,32 @@ 3600000 3600000 3600000 + + 6 diff --git a/backend-api/src/main/java/com/guildworkman/api/booking/service/SlotReservationService.java b/backend-api/src/main/java/com/guildworkman/api/booking/service/SlotReservationService.java index b73e691..3384f37 100644 --- a/backend-api/src/main/java/com/guildworkman/api/booking/service/SlotReservationService.java +++ b/backend-api/src/main/java/com/guildworkman/api/booking/service/SlotReservationService.java @@ -9,6 +9,7 @@ import com.guildworkman.api.booking.model.SlotReservationStatus; import com.guildworkman.api.booking.repository.SlotReservationRepository; import com.guildworkman.api.data.constants.AppointmentStatus; +import com.guildworkman.api.data.constants.NotificationType; import com.guildworkman.api.data.models.Appointment; import com.guildworkman.api.data.models.Client; import com.guildworkman.api.data.models.SkilledWorker; @@ -17,6 +18,7 @@ import com.guildworkman.api.data.repository.SkilledWorkerRepository; import com.guildworkman.api.exceptions.GuildWorkmanException; import com.guildworkman.api.exceptions.UserNotFoundException; +import com.guildworkman.api.services.ServiceUtils.NotificationService; import lombok.RequiredArgsConstructor; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -71,6 +73,7 @@ public class SlotReservationService { private final ClientRepository clients; private final SkilledWorkerRepository skilledWorkers; private final SlotReservationProperties properties; + private final NotificationService notificationService; /** @param replayed true when {@code reservation} already existed for this idempotency key. */ public record ReservationOutcome(SlotReservation reservation, boolean replayed) { @@ -159,6 +162,8 @@ public SlotReservation confirm(Long reservationId, ConfirmReservationRequest req reservation.setAppointmentId(appointment.getId()); reservations.save(reservation); + notificationService.notifyAppointmentEvent(appointment, NotificationType.APPOINTMENT_BOOKED); + log.info("Slot reservation id={} confirmed as appointment id={}", reservationId, appointment.getId()); return reservation; } diff --git a/backend-api/src/main/java/com/guildworkman/api/config/AsyncConfig.java b/backend-api/src/main/java/com/guildworkman/api/config/AsyncConfig.java new file mode 100644 index 0000000..6b64862 --- /dev/null +++ b/backend-api/src/main/java/com/guildworkman/api/config/AsyncConfig.java @@ -0,0 +1,33 @@ +package com.guildworkman.api.config; + +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.scheduling.annotation.EnableAsync; +import org.springframework.scheduling.concurrent.ThreadPoolTaskExecutor; + +import java.util.concurrent.Executor; + +/** + * Backing executor for the notification fan-out email. Dedicated and small on + * purpose: mail-provider calls are the one thing in this app that must never + * share a thread pool with request handling, since a slow or down provider + * (see {@code NotificationEmailDispatcher}) must not be able to starve + * anything else. + */ +@Configuration +@EnableAsync +public class AsyncConfig { + + public static final String NOTIFICATION_MAIL_EXECUTOR = "notificationMailExecutor"; + + @Bean(NOTIFICATION_MAIL_EXECUTOR) + public Executor notificationMailExecutor() { + ThreadPoolTaskExecutor executor = new ThreadPoolTaskExecutor(); + executor.setCorePoolSize(2); + executor.setMaxPoolSize(8); + executor.setQueueCapacity(500); + executor.setThreadNamePrefix("notif-mail-"); + executor.initialize(); + return executor; + } +} diff --git a/backend-api/src/main/java/com/guildworkman/api/controllers/NotificationController.java b/backend-api/src/main/java/com/guildworkman/api/controllers/NotificationController.java new file mode 100644 index 0000000..61eb884 --- /dev/null +++ b/backend-api/src/main/java/com/guildworkman/api/controllers/NotificationController.java @@ -0,0 +1,64 @@ +package com.guildworkman.api.controllers; + +import com.guildworkman.api.dto.responses.NotificationResponse; +import com.guildworkman.api.dto.responses.UnreadCountResponse; +import com.guildworkman.api.services.ServiceUtils.NotificationService; +import io.swagger.v3.oas.annotations.Operation; +import io.swagger.v3.oas.annotations.security.SecurityRequirement; +import lombok.RequiredArgsConstructor; +import org.springframework.data.domain.Pageable; +import org.springframework.data.domain.Sort; +import org.springframework.data.web.PageableDefault; +import org.springframework.data.web.PagedModel; +import org.springframework.security.core.annotation.AuthenticationPrincipal; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.PutMapping; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RestController; + +import java.util.List; + +/** + * A user's own in-app notifications. Every endpoint scopes to the calling + * user's email (the JWT principal — see {@code JwtAuthenticationFilter}), so + * there is no id-based lookup that could leak another user's notifications; + * {@code markAsRead} 404s on a notification id that exists but belongs to + * someone else, rather than exposing it. + */ +@RestController +@RequestMapping("/api/v1/notifications") +@RequiredArgsConstructor +@SecurityRequirement(name = "bearerAuth") +public class NotificationController { + + private final NotificationService notificationService; + + @GetMapping + @Operation(summary = "List the caller's notifications, newest first", description = + "Paginated. Default page size 20; override with the standard page/size/sort query params.") + public PagedModel list( + @AuthenticationPrincipal String recipientEmail, + @PageableDefault(size = 20, sort = "createdAt", direction = Sort.Direction.DESC) Pageable pageable) { + return new PagedModel<>(notificationService.getNotifications(recipientEmail, pageable)); + } + + @GetMapping("/unread-count") + @Operation(summary = "Count of the caller's unread notifications") + public UnreadCountResponse unreadCount(@AuthenticationPrincipal String recipientEmail) { + return new UnreadCountResponse(notificationService.countUnread(recipientEmail)); + } + + @PutMapping("/{id}/read") + @Operation(summary = "Mark one notification read", description = + "404 if the notification doesn't exist or doesn't belong to the caller.") + public NotificationResponse markAsRead(@AuthenticationPrincipal String recipientEmail, @PathVariable Long id) { + return notificationService.markAsRead(id, recipientEmail); + } + + @PutMapping("/read-all") + @Operation(summary = "Mark all of the caller's unread notifications read") + public List markAllAsRead(@AuthenticationPrincipal String recipientEmail) { + return notificationService.markAllAsRead(recipientEmail); + } +} diff --git a/backend-api/src/main/java/com/guildworkman/api/data/constants/NotificationEmailStatus.java b/backend-api/src/main/java/com/guildworkman/api/data/constants/NotificationEmailStatus.java new file mode 100644 index 0000000..c6897f1 --- /dev/null +++ b/backend-api/src/main/java/com/guildworkman/api/data/constants/NotificationEmailStatus.java @@ -0,0 +1,13 @@ +package com.guildworkman.api.data.constants; + +/** + * Tracks the fan-out email for a notification, independent of the + * notification row itself: the in-app notification is always persisted, but + * the email is sent asynchronously and can fail without affecting it. See + * {@code NotificationEmailDispatcher}. + */ +public enum NotificationEmailStatus { + PENDING, + SENT, + FAILED +} diff --git a/backend-api/src/main/java/com/guildworkman/api/data/constants/NotificationType.java b/backend-api/src/main/java/com/guildworkman/api/data/constants/NotificationType.java new file mode 100644 index 0000000..a244084 --- /dev/null +++ b/backend-api/src/main/java/com/guildworkman/api/data/constants/NotificationType.java @@ -0,0 +1,41 @@ +package com.guildworkman.api.data.constants; + +/** + * The appointment lifecycle transitions that produce a notification. Each + * value carries the title used for both the in-app notification and the + * fan-out email's subject line. + */ +public enum NotificationType { + APPOINTMENT_BOOKED("Appointment booked"), + APPOINTMENT_ACCEPTED("Appointment accepted"), + APPOINTMENT_DECLINED("Appointment declined"), + APPOINTMENT_UPDATED("Appointment updated"), + APPOINTMENT_CANCELLED("Appointment cancelled"), + APPOINTMENT_DELETED("Appointment removed"); + + private final String title; + + NotificationType(String title) { + this.title = title; + } + + public String title() { + return title; + } + + /** + * Maps the status an appointment was just saved with to the notification it + * produces. Used by {@code updateAppointment}, which applies whatever status + * the caller asked for (ACCEPTED/DECLINED/CANCELLED/UPDATED) through a + * single endpoint. + */ + public static NotificationType fromAppointmentStatus(AppointmentStatus status) { + return switch (status) { + case ACCEPTED -> APPOINTMENT_ACCEPTED; + case DECLINED -> APPOINTMENT_DECLINED; + case CANCELLED -> APPOINTMENT_CANCELLED; + case SCHEDULED -> APPOINTMENT_BOOKED; + case UPDATED -> APPOINTMENT_UPDATED; + }; + } +} diff --git a/backend-api/src/main/java/com/guildworkman/api/data/models/Notification.java b/backend-api/src/main/java/com/guildworkman/api/data/models/Notification.java index d4d6742..2d3d0e4 100644 --- a/backend-api/src/main/java/com/guildworkman/api/data/models/Notification.java +++ b/backend-api/src/main/java/com/guildworkman/api/data/models/Notification.java @@ -1,20 +1,94 @@ package com.guildworkman.api.data.models; +import com.guildworkman.api.data.constants.NotificationEmailStatus; +import com.guildworkman.api.data.constants.NotificationType; +import com.guildworkman.api.data.constants.Role; +import jakarta.persistence.Column; import jakarta.persistence.Entity; +import jakarta.persistence.EnumType; +import jakarta.persistence.Enumerated; import jakarta.persistence.GeneratedValue; import jakarta.persistence.GenerationType; import jakarta.persistence.Id; +import jakarta.persistence.Index; +import jakarta.persistence.Table; import lombok.Getter; import lombok.Setter; +import java.time.Instant; + +/** + * A persisted, readable in-app notification produced by an appointment + * lifecycle transition (booked/accepted/declined/updated/cancelled/deleted). + * + *

{@code recipientEmail} is the scoping key for "a user's own + * notifications": it is the one identifier shared across {@link UserAccount}, + * {@link Client} and {@link SkilledWorker} in this codebase (they are not yet + * linked by a foreign key — see {@link UserAccount}'s class Javadoc), and it + * is what {@code JwtAuthenticationFilter} puts in the security principal. + * + *

The primary key column keeps its original name ({@code notification_id}) + * even though every other field here is new: this table already exists (as + * an unused stub), and this codebase manages schema via + * {@code spring.jpa.hibernate.ddl-auto=update}, which only ever adds missing + * columns — it does not rename or drop them. Renaming the identity column + * would leave the old primary key behind and could fail to apply cleanly on + * an already-deployed database. See {@code docs/NOTIFICATION_SERVICE.md}. + * + *

Deliberately no {@code @Version}: the only mutation after insert is + * {@code read} flipping {@code false → true} (plus {@code emailStatus}, set + * exactly once by {@code NotificationEmailDispatcher}), and every writer + * scopes by {@code recipientEmail} first — see + * {@code NotificationServiceImpl#markAsRead}/{@code #markAllAsRead}. Two + * concurrent mark-read calls on the same row (a double-tap, or list + mark-all + * racing) both want the same end state, so optimistic locking would only add + * a spurious {@code OptimisticLockException} on an outcome that was already + * correct, not prevent a lost update. + */ @Entity +@Table(name = "notifications", indexes = { + @Index(name = "idx_notifications_recipient_created", columnList = "recipient_email,created_at"), + @Index(name = "idx_notifications_recipient_unread", columnList = "recipient_email,is_read") +}) @Getter @Setter public class Notification { + @Id @GeneratedValue(strategy = GenerationType.IDENTITY) private Long notificationId; - private Long userId; + + @Column(name = "recipient_email", nullable = false, updatable = false) + private String recipientEmail; + + @Enumerated(EnumType.STRING) + @Column(name = "recipient_role", length = 20, updatable = false) + private Role recipientRole; + + @Enumerated(EnumType.STRING) + @Column(name = "type", length = 40, updatable = false) + private NotificationType type; + + @Column(name = "title", length = 200, updatable = false) + private String title; + + @Column(name = "message", length = 1000, updatable = false) private String message; - private String Status; + + /** Not a JPA foreign key on purpose: the appointment may since be deleted. */ + @Column(name = "appointment_id", updatable = false) + private Long appointmentId; + + @Column(name = "is_read", nullable = false) + private boolean read; + + @Column(name = "read_at") + private Instant readAt; + + @Enumerated(EnumType.STRING) + @Column(name = "email_status", length = 20) + private NotificationEmailStatus emailStatus = NotificationEmailStatus.PENDING; + + @Column(name = "created_at", updatable = false) + private Instant createdAt = Instant.now(); } diff --git a/backend-api/src/main/java/com/guildworkman/api/data/repository/NotificationRepository.java b/backend-api/src/main/java/com/guildworkman/api/data/repository/NotificationRepository.java index a3aff47..5f62b6f 100644 --- a/backend-api/src/main/java/com/guildworkman/api/data/repository/NotificationRepository.java +++ b/backend-api/src/main/java/com/guildworkman/api/data/repository/NotificationRepository.java @@ -1,7 +1,20 @@ package com.guildworkman.api.data.repository; import com.guildworkman.api.data.models.Notification; +import org.springframework.data.domain.Page; +import org.springframework.data.domain.Pageable; import org.springframework.data.jpa.repository.JpaRepository; +import java.util.List; +import java.util.Optional; + public interface NotificationRepository extends JpaRepository { + + Page findByRecipientEmail(String recipientEmail, Pageable pageable); + + long countByRecipientEmailAndReadFalse(String recipientEmail); + + Optional findByNotificationIdAndRecipientEmail(Long notificationId, String recipientEmail); + + List findByRecipientEmailAndReadFalse(String recipientEmail); } diff --git a/backend-api/src/main/java/com/guildworkman/api/dto/responses/NotificationResponse.java b/backend-api/src/main/java/com/guildworkman/api/dto/responses/NotificationResponse.java new file mode 100644 index 0000000..18310eb --- /dev/null +++ b/backend-api/src/main/java/com/guildworkman/api/dto/responses/NotificationResponse.java @@ -0,0 +1,29 @@ +package com.guildworkman.api.dto.responses; + +import com.guildworkman.api.data.constants.NotificationType; +import com.guildworkman.api.data.models.Notification; + +import java.time.Instant; + +public record NotificationResponse( + Long id, + NotificationType type, + String title, + String message, + Long appointmentId, + boolean read, + Instant readAt, + Instant createdAt +) { + public static NotificationResponse from(Notification notification) { + return new NotificationResponse( + notification.getNotificationId(), + notification.getType(), + notification.getTitle(), + notification.getMessage(), + notification.getAppointmentId(), + notification.isRead(), + notification.getReadAt(), + notification.getCreatedAt()); + } +} diff --git a/backend-api/src/main/java/com/guildworkman/api/dto/responses/UnreadCountResponse.java b/backend-api/src/main/java/com/guildworkman/api/dto/responses/UnreadCountResponse.java new file mode 100644 index 0000000..7d5a18d --- /dev/null +++ b/backend-api/src/main/java/com/guildworkman/api/dto/responses/UnreadCountResponse.java @@ -0,0 +1,4 @@ +package com.guildworkman.api.dto.responses; + +public record UnreadCountResponse(long unread) { +} diff --git a/backend-api/src/main/java/com/guildworkman/api/exceptions/NotificationNotFoundException.java b/backend-api/src/main/java/com/guildworkman/api/exceptions/NotificationNotFoundException.java new file mode 100644 index 0000000..6e2ce19 --- /dev/null +++ b/backend-api/src/main/java/com/guildworkman/api/exceptions/NotificationNotFoundException.java @@ -0,0 +1,7 @@ +package com.guildworkman.api.exceptions; + +public class NotificationNotFoundException extends RuntimeException { + public NotificationNotFoundException(String e) { + super(e); + } +} diff --git a/backend-api/src/main/java/com/guildworkman/api/handler/GlobalExceptionHandler.java b/backend-api/src/main/java/com/guildworkman/api/handler/GlobalExceptionHandler.java index 273c67c..d414c10 100644 --- a/backend-api/src/main/java/com/guildworkman/api/handler/GlobalExceptionHandler.java +++ b/backend-api/src/main/java/com/guildworkman/api/handler/GlobalExceptionHandler.java @@ -102,6 +102,12 @@ public ResponseEntity handleAppointmentNotFound(AppointmentNotFou "Appointment not found", exception.getMessage()); } + @ExceptionHandler(NotificationNotFoundException.class) + public ResponseEntity handleNotificationNotFound(NotificationNotFoundException exception) { + return respond(HttpStatus.NOT_FOUND, "notification-not-found", + "Notification not found", exception.getMessage()); + } + @ExceptionHandler(ReservationNotFoundException.class) public ResponseEntity handleReservationNotFound(ReservationNotFoundException exception) { return respond(HttpStatus.NOT_FOUND, "reservation-not-found", diff --git a/backend-api/src/main/java/com/guildworkman/api/services/ServiceUtils/NotificationService.java b/backend-api/src/main/java/com/guildworkman/api/services/ServiceUtils/NotificationService.java index 3dc874b..d8e7545 100644 --- a/backend-api/src/main/java/com/guildworkman/api/services/ServiceUtils/NotificationService.java +++ b/backend-api/src/main/java/com/guildworkman/api/services/ServiceUtils/NotificationService.java @@ -1,3 +1,31 @@ package com.guildworkman.api.services.ServiceUtils; + +import com.guildworkman.api.data.constants.NotificationType; +import com.guildworkman.api.data.models.Appointment; +import com.guildworkman.api.dto.responses.NotificationResponse; +import org.springframework.data.domain.Page; +import org.springframework.data.domain.Pageable; + +import java.util.List; + public interface NotificationService { + + /** + * Persists a notification for whichever of the appointment's client/worker + * the given lifecycle event concerns, and schedules the corresponding + * fan-out email for each. Never rolls back or blocks the caller: the email + * send happens after the enclosing transaction commits (if any) on a + * separate thread, and a failed send only marks that notification's email + * status — it never propagates back here. + */ + void notifyAppointmentEvent(Appointment appointment, NotificationType type); + + Page getNotifications(String recipientEmail, Pageable pageable); + + long countUnread(String recipientEmail); + + /** @throws com.guildworkman.api.exceptions.NotificationNotFoundException if it doesn't exist or isn't the caller's */ + NotificationResponse markAsRead(Long notificationId, String recipientEmail); + + List markAllAsRead(String recipientEmail); } diff --git a/backend-api/src/main/java/com/guildworkman/api/services/implmentations/AppointmentServiceImpl.java b/backend-api/src/main/java/com/guildworkman/api/services/implmentations/AppointmentServiceImpl.java index e88862b..6cebe02 100644 --- a/backend-api/src/main/java/com/guildworkman/api/services/implmentations/AppointmentServiceImpl.java +++ b/backend-api/src/main/java/com/guildworkman/api/services/implmentations/AppointmentServiceImpl.java @@ -7,10 +7,13 @@ import com.guildworkman.api.dto.responses.UpdateAppointmentResponse; import com.guildworkman.api.dto.responses.ViewAllAppointmentsResponse; import com.guildworkman.api.exceptions.AppointmentNotFoundException; +import com.guildworkman.api.data.constants.NotificationType; import com.guildworkman.api.data.models.Appointment; +import com.guildworkman.api.data.models.Client; import com.guildworkman.api.data.models.SkilledWorker; import com.guildworkman.api.data.constants.AppointmentStatus; import com.guildworkman.api.data.repository.AppointmentRepository; +import com.guildworkman.api.data.repository.ClientRepository; import com.guildworkman.api.data.repository.SkilledWorkerRepository; import com.guildworkman.api.exceptions.GuildWorkmanException; import com.guildworkman.api.exceptions.UserNotFoundException; @@ -18,6 +21,7 @@ import com.guildworkman.api.booking.service.SlotReservationBooker; import com.guildworkman.api.booking.service.SlotReservationService; import com.guildworkman.api.services.ServiceUtils.AppointmentService; +import com.guildworkman.api.services.ServiceUtils.NotificationService; import lombok.AllArgsConstructor; import org.modelmapper.ModelMapper; import org.springframework.stereotype.Service; @@ -46,8 +50,10 @@ public class AppointmentServiceImpl implements AppointmentService { private final AppointmentRepository appointmentRepository; private final ModelMapper modelMapper; private final SkilledWorkerRepository skilledWorkerRepository; + private final ClientRepository clientRepository; private final SlotReservationBooker slotReservationBooker; private final SlotReservationService slotReservationService; + private final NotificationService notificationService; // @Autowired @@ -69,6 +75,15 @@ public Appointment bookAppointment(BookAppointmentRequest bookAppointmentRequest appointment.setSkilledWorker(skilledWorker); appointment.setAmount(bookAppointmentRequest.getAmount()); + // Same reasoning as the worker above: ModelMapper maps `clientId` onto a + // transient Client with only its id set (everything else null), which + // would either fail to persist or silently notify with a blank name/email. + // Resolve the managed entity instead. + Client client = clientRepository.findById(bookAppointmentRequest.getClientId()) + .orElseThrow(() -> new UserNotFoundException( + "Client not found: " + bookAppointmentRequest.getClientId())); + appointment.setClient(client); + // Claim the slot BEFORE the appointment exists. This one-step endpoint // races just as hard as the two-step reserve/confirm flow does, so it // goes through the same per-worker lock and overlap check — otherwise @@ -92,6 +107,9 @@ public Appointment bookAppointment(BookAppointmentRequest bookAppointmentRequest if (reservation != null) { slotReservationBooker.linkAppointment(reservation, appointment.getId()); } + + notificationService.notifyAppointmentEvent(appointment, NotificationType.APPOINTMENT_BOOKED); + return appointment; } @@ -136,11 +154,13 @@ private void moveToSlot(Appointment appointment, LocalDateTime newStart) { @Override + @Transactional public void cancelAppointment(Long id) { Appointment appointment = appointmentRepository.findById(id) .orElseThrow(() -> new AppointmentNotFoundException("Appointment not found")); appointment.setStatus(AppointmentStatus.CANCELLED); appointmentRepository.save(appointment); + notificationService.notifyAppointmentEvent(appointment, NotificationType.APPOINTMENT_CANCELLED); // CancelAppointmentResponse response = new CancelAppointmentResponse(); // modelMapper.map(response, Appointment.class); // response.setMessage("Appointment cancelled successfully"); @@ -183,6 +203,9 @@ public UpdateAppointmentResponse updateAppointment(Long Id,UpdateAppointmentRequ } appointmentRepository.save(appointment); + notificationService.notifyAppointmentEvent(appointment, + NotificationType.fromAppointmentStatus(appointment.getStatus())); + UpdateAppointmentResponse response = new UpdateAppointmentResponse(); response.setId(appointment.getId()); response.setMessage("Appointment Updated"); @@ -190,10 +213,12 @@ public UpdateAppointmentResponse updateAppointment(Long Id,UpdateAppointmentRequ } @Override + @Transactional public void deleteAppointment(Long id) { Appointment appointment = appointmentRepository.findById(id) .orElseThrow(()-> new AppointmentNotFoundException("No appointment found")); appointmentRepository.delete(appointment); + notificationService.notifyAppointmentEvent(appointment, NotificationType.APPOINTMENT_DELETED); } @@ -221,6 +246,7 @@ public void save(Appointment appointment) { } @Override + @Transactional public AcceptAppointmentResponse acceptAppointment(AcceptAppointmentRequest request) { Appointment appointment = appointmentRepository.findById(request.getAppointmentId()) .orElseThrow(()-> new GuildWorkmanException("appointment not found")); @@ -228,6 +254,7 @@ public AcceptAppointmentResponse acceptAppointment(AcceptAppointmentRequest requ // appointment.setSkilledWorker(request.getSkilledWorker()); appointment.setStatus(AppointmentStatus.ACCEPTED); appointmentRepository.save(appointment); + notificationService.notifyAppointmentEvent(appointment, NotificationType.APPOINTMENT_ACCEPTED); AcceptAppointmentResponse response = new AcceptAppointmentResponse(); response.setStatus(request.getStatus()); response.setClientId(request.getId()); diff --git a/backend-api/src/main/java/com/guildworkman/api/services/implmentations/NotificationEmailDispatcher.java b/backend-api/src/main/java/com/guildworkman/api/services/implmentations/NotificationEmailDispatcher.java new file mode 100644 index 0000000..24202bd --- /dev/null +++ b/backend-api/src/main/java/com/guildworkman/api/services/implmentations/NotificationEmailDispatcher.java @@ -0,0 +1,75 @@ +package com.guildworkman.api.services.implmentations; + +import com.guildworkman.api.config.AsyncConfig; +import com.guildworkman.api.data.constants.NotificationEmailStatus; +import com.guildworkman.api.data.repository.NotificationRepository; +import com.guildworkman.api.dto.requests.SendMailRequest; +import com.guildworkman.api.services.ServiceUtils.MailService; +import lombok.RequiredArgsConstructor; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.scheduling.annotation.Async; +import org.springframework.stereotype.Service; + +/** + * Sends the transactional email behind one notification, off the request + * thread and after the appointment transaction that produced it has already + * committed (see {@code NotificationServiceImpl#scheduleEmail}). + * + *

A separate bean from {@code NotificationServiceImpl} is required for + * {@code @Async} to actually apply: Spring's async proxy only intercepts + * calls that arrive from outside the bean, so a self-invoked + * {@code @Async} method on the same class would run synchronously. + * + *

{@code MailService} failures (a down or slow provider, a non-2xx + * response) are caught here and never rethrown: this runs with nothing left + * to observe the exception, and the whole point is that a mail failure must + * not affect the appointment operation that already succeeded. The outcome is + * instead recorded on the notification's {@code emailStatus} and logged at + * {@code ERROR}. + * + *

Observability: a failure is currently only surfaced as a log line + * plus the persisted {@code emailStatus=FAILED} row (queryable directly, or + * via log-based alerting on this class). There is no emitted metric/counter — + * this codebase has no Micrometer/Actuator on the classpath today, and adding + * one is a new dependency + cross-cutting change worth its own PR rather than + * folding it into this one. See "Observability" in + * {@code docs/NOTIFICATION_SERVICE.md}. + */ +@Service +@RequiredArgsConstructor +public class NotificationEmailDispatcher { + + private static final Logger log = LoggerFactory.getLogger(NotificationEmailDispatcher.class); + + private final MailService mailService; + private final NotificationRepository notificationRepository; + + @Async(AsyncConfig.NOTIFICATION_MAIL_EXECUTOR) + public void dispatch(Long notificationId, String recipientEmail, String recipientName, + String subject, String htmlContent) { + try { + SendMailRequest request = new SendMailRequest(); + request.setRecipientEmail(recipientEmail); + request.setRecipientName(recipientName); + request.setSubject(subject); + request.setContent(htmlContent); + mailService.sendMail(request); + markEmailStatus(notificationId, NotificationEmailStatus.SENT); + } catch (Exception exception) { + // ERROR, not WARN: this is the one outcome ops needs to be able to + // alert on from logs alone — there's no metrics counter for it (see + // class Javadoc), so the log line is the only signal that exists. + log.error("Failed to send notification email (notificationId={}, recipient={}): {}", + notificationId, recipientEmail, exception.getMessage(), exception); + markEmailStatus(notificationId, NotificationEmailStatus.FAILED); + } + } + + private void markEmailStatus(Long notificationId, NotificationEmailStatus status) { + notificationRepository.findById(notificationId).ifPresent(notification -> { + notification.setEmailStatus(status); + notificationRepository.save(notification); + }); + } +} diff --git a/backend-api/src/main/java/com/guildworkman/api/services/implmentations/NotificationServiceImpl.java b/backend-api/src/main/java/com/guildworkman/api/services/implmentations/NotificationServiceImpl.java index c7320b0..3c7ddff 100644 --- a/backend-api/src/main/java/com/guildworkman/api/services/implmentations/NotificationServiceImpl.java +++ b/backend-api/src/main/java/com/guildworkman/api/services/implmentations/NotificationServiceImpl.java @@ -1,8 +1,168 @@ package com.guildworkman.api.services.implmentations; +import com.guildworkman.api.data.constants.NotificationType; +import com.guildworkman.api.data.constants.Role; +import com.guildworkman.api.data.models.Appointment; +import com.guildworkman.api.data.models.Client; +import com.guildworkman.api.data.models.Notification; +import com.guildworkman.api.data.models.SkilledWorker; +import com.guildworkman.api.data.repository.NotificationRepository; +import com.guildworkman.api.dto.responses.NotificationResponse; +import com.guildworkman.api.exceptions.NotificationNotFoundException; import com.guildworkman.api.services.ServiceUtils.NotificationService; +import lombok.RequiredArgsConstructor; +import org.springframework.data.domain.Page; +import org.springframework.data.domain.Pageable; import org.springframework.stereotype.Service; +import org.springframework.transaction.support.TransactionSynchronization; +import org.springframework.transaction.support.TransactionSynchronizationManager; + +import java.time.Instant; +import java.util.ArrayList; +import java.util.List; @Service +@RequiredArgsConstructor public class NotificationServiceImpl implements NotificationService { + + private final NotificationRepository notificationRepository; + private final NotificationEmailDispatcher emailDispatcher; + + private record Recipient(String email, String name, Role role) { + } + + @Override + public void notifyAppointmentEvent(Appointment appointment, NotificationType type) { + for (Recipient recipient : recipientsFor(appointment, type)) { + Notification notification = new Notification(); + notification.setRecipientEmail(recipient.email()); + notification.setRecipientRole(recipient.role()); + notification.setType(type); + notification.setTitle(type.title()); + notification.setMessage(buildMessage(appointment, type, recipient.role())); + notification.setAppointmentId(appointment.getId()); + notification = notificationRepository.save(notification); + + scheduleEmail(notification, recipient.name()); + } + } + + @Override + public Page getNotifications(String recipientEmail, Pageable pageable) { + return notificationRepository.findByRecipientEmail(recipientEmail, pageable) + .map(NotificationResponse::from); + } + + @Override + public long countUnread(String recipientEmail) { + return notificationRepository.countByRecipientEmailAndReadFalse(recipientEmail); + } + + @Override + public NotificationResponse markAsRead(Long notificationId, String recipientEmail) { + Notification notification = notificationRepository + .findByNotificationIdAndRecipientEmail(notificationId, recipientEmail) + .orElseThrow(() -> new NotificationNotFoundException("Notification not found: " + notificationId)); + + if (!notification.isRead()) { + notification.setRead(true); + notification.setReadAt(Instant.now()); + notification = notificationRepository.save(notification); + } + return NotificationResponse.from(notification); + } + + @Override + public List markAllAsRead(String recipientEmail) { + List unread = notificationRepository.findByRecipientEmailAndReadFalse(recipientEmail); + Instant now = Instant.now(); + List responses = new ArrayList<>(unread.size()); + for (Notification notification : unread) { + notification.setRead(true); + notification.setReadAt(now); + responses.add(NotificationResponse.from(notificationRepository.save(notification))); + } + return responses; + } + + // --- recipients & content ------------------------------------------------- + + /** + * ACCEPTED/DECLINED are the worker's own decision on the client's request, + * so only the client is told; every other transition affects both sides of + * the appointment. + */ + private List recipientsFor(Appointment appointment, NotificationType type) { + List recipients = new ArrayList<>(); + Client client = appointment.getClient(); + if (client != null && client.getEmail() != null) { + recipients.add(new Recipient(client.getEmail(), client.getFullName(), Role.CLIENT)); + } + + boolean notifyWorker = type != NotificationType.APPOINTMENT_ACCEPTED + && type != NotificationType.APPOINTMENT_DECLINED; + SkilledWorker worker = appointment.getSkilledWorker(); + if (notifyWorker && worker != null && worker.getEmail() != null) { + recipients.add(new Recipient(worker.getEmail(), worker.getFullName(), Role.SKILLED_WORKER)); + } + return recipients; + } + + private String buildMessage(Appointment appointment, NotificationType type, Role recipientRole) { + String counterpart = recipientRole == Role.CLIENT + ? nameOrDefault(appointment.getSkilledWorker() != null ? appointment.getSkilledWorker().getFullName() : null, "a worker") + : nameOrDefault(appointment.getClient() != null ? appointment.getClient().getFullName() : null, "a client"); + String when = appointment.getScheduleTime() != null + ? appointment.getScheduleTime().toString() + : "an unspecified time"; + + return switch (type) { + case APPOINTMENT_BOOKED -> recipientRole == Role.CLIENT + ? "Your appointment with %s on %s has been booked.".formatted(counterpart, when) + : "You have a new appointment request from %s on %s.".formatted(counterpart, when); + case APPOINTMENT_ACCEPTED -> "Your appointment with %s on %s was accepted.".formatted(counterpart, when); + case APPOINTMENT_DECLINED -> "Your appointment with %s on %s was declined.".formatted(counterpart, when); + case APPOINTMENT_UPDATED -> "Your appointment with %s on %s was updated.".formatted(counterpart, when); + case APPOINTMENT_CANCELLED -> "Your appointment with %s on %s was cancelled.".formatted(counterpart, when); + case APPOINTMENT_DELETED -> "Your appointment with %s on %s was removed.".formatted(counterpart, when); + }; + } + + private String nameOrDefault(String name, String fallback) { + return (name == null || name.isBlank()) ? fallback : name; + } + + // --- email fan-out ---------------------------------------------------- + + /** + * Defers the actual send until the enclosing transaction (the appointment + * write that triggered this notification) has committed, so a notification + * is never emailed for a change that ends up rolled back. If there is no + * active transaction (e.g. called outside one), the send is scheduled + * immediately; either way {@link NotificationEmailDispatcher#dispatch} runs + * asynchronously and never on this thread. + */ + private void scheduleEmail(Notification notification, String recipientName) { + Long notificationId = notification.getNotificationId(); + String recipientEmail = notification.getRecipientEmail(); + String subject = notification.getTitle(); + String content = renderEmailHtml(notification.getTitle(), notification.getMessage()); + + Runnable send = () -> emailDispatcher.dispatch(notificationId, recipientEmail, recipientName, subject, content); + + if (TransactionSynchronizationManager.isSynchronizationActive()) { + TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() { + @Override + public void afterCommit() { + send.run(); + } + }); + } else { + send.run(); + } + } + + private String renderEmailHtml(String title, String message) { + return "

%s

%s

".formatted(title, message); + } } diff --git a/backend-api/src/main/resources/application.properties b/backend-api/src/main/resources/application.properties index 4c156f6..f3fa6b7 100644 --- a/backend-api/src/main/resources/application.properties +++ b/backend-api/src/main/resources/application.properties @@ -171,6 +171,12 @@ security.jwt.refresh-token-ttl=${JWT_REFRESH_TTL:P7D} # local development and the live frontend (guildworkman-web.vercel.app). cors.allowed-origins=${CORS_ALLOWED_ORIGINS:http://localhost:3000,https://guildworkman-web.vercel.app} +# Hard cap on the page size any Pageable-bound endpoint accepts (currently +# only GET /api/v1/notifications), regardless of what a caller passes as +# ?size=. Spring Data's own default (2000) is too high to call "sane" for a +# public-facing list endpoint; NotificationController's own @PageableDefault +# (20) only sets what happens when size is omitted, it doesn't bound it. +spring.data.web.pageable.max-page-size=${PAGEABLE_MAX_PAGE_SIZE:100} # --- Metrics --------------------------------------------------------------- # Micrometer counters for the signing pipeline (see signing/service/SigningMetrics) # are scraped from /actuator/prometheus. Only health, info and prometheus are diff --git a/backend-api/src/test/java/com/guildworkman/api/booking/SlotReservationIntegrationTest.java b/backend-api/src/test/java/com/guildworkman/api/booking/SlotReservationIntegrationTest.java index d47ffd0..7340ff2 100644 --- a/backend-api/src/test/java/com/guildworkman/api/booking/SlotReservationIntegrationTest.java +++ b/backend-api/src/test/java/com/guildworkman/api/booking/SlotReservationIntegrationTest.java @@ -22,10 +22,12 @@ import com.guildworkman.api.dto.requests.UpdateAppointmentRequest; import com.guildworkman.api.exceptions.UserNotFoundException; import com.guildworkman.api.services.ServiceUtils.ClientService; +import com.guildworkman.api.services.ServiceUtils.MailService; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.mock.mockito.MockBean; import java.math.BigDecimal; import java.time.Duration; @@ -83,6 +85,12 @@ class SlotReservationIntegrationTest { @Autowired private ClientService clientService; + // Booking/confirming an appointment now fans out a notification email. + // Mocked so these tests never make a real call to the mail provider, + // mirroring how EscrowOrchestrationIntegrationTest mocks SorobanRpcClient. + @MockBean + private MailService mailService; + private SkilledWorker worker; private Client client; private LocalDateTime slotStart; diff --git a/backend-api/src/test/java/com/guildworkman/api/booking/api/BookingControllerTest.java b/backend-api/src/test/java/com/guildworkman/api/booking/api/BookingControllerTest.java index 4c8fa53..9c78793 100644 --- a/backend-api/src/test/java/com/guildworkman/api/booking/api/BookingControllerTest.java +++ b/backend-api/src/test/java/com/guildworkman/api/booking/api/BookingControllerTest.java @@ -10,11 +10,13 @@ import com.guildworkman.api.data.repository.ClientRepository; import com.guildworkman.api.data.repository.SkilledWorkerRepository; import com.guildworkman.api.handler.ProblemDetails; +import com.guildworkman.api.services.ServiceUtils.MailService; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.mock.mockito.MockBean; import org.springframework.http.MediaType; import org.springframework.test.web.servlet.MockMvc; @@ -65,6 +67,11 @@ class BookingControllerTest { @Autowired private SkilledWorkerRepository skilledWorkers; + // Confirming a reservation now fans out a notification email; mocked so + // this never makes a real call to the mail provider. + @MockBean + private MailService mailService; + private SkilledWorker worker; private Client client; private LocalDateTime slotStart; diff --git a/backend-api/src/test/java/com/guildworkman/api/booking/service/SlotReservationServiceTest.java b/backend-api/src/test/java/com/guildworkman/api/booking/service/SlotReservationServiceTest.java index c61c3d0..6900a6c 100644 --- a/backend-api/src/test/java/com/guildworkman/api/booking/service/SlotReservationServiceTest.java +++ b/backend-api/src/test/java/com/guildworkman/api/booking/service/SlotReservationServiceTest.java @@ -7,6 +7,7 @@ import com.guildworkman.api.data.repository.AppointmentRepository; import com.guildworkman.api.data.repository.ClientRepository; import com.guildworkman.api.data.repository.SkilledWorkerRepository; +import com.guildworkman.api.services.ServiceUtils.NotificationService; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -50,6 +51,8 @@ class SlotReservationServiceTest { private SkilledWorkerRepository skilledWorkers; @Mock private SlotReservationProperties properties; + @Mock + private NotificationService notificationService; @InjectMocks private SlotReservationService service; diff --git a/backend-api/src/test/java/com/guildworkman/api/controllers/NotificationControllerTest.java b/backend-api/src/test/java/com/guildworkman/api/controllers/NotificationControllerTest.java new file mode 100644 index 0000000..05de7be --- /dev/null +++ b/backend-api/src/test/java/com/guildworkman/api/controllers/NotificationControllerTest.java @@ -0,0 +1,144 @@ +package com.guildworkman.api.controllers; + +import com.guildworkman.api.data.constants.NotificationType; +import com.guildworkman.api.data.constants.Role; +import com.guildworkman.api.data.models.Notification; +import com.guildworkman.api.data.models.UserAccount; +import com.guildworkman.api.data.repository.NotificationRepository; +import com.guildworkman.api.handler.ProblemDetails; +import com.guildworkman.api.security.JwtService; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.test.web.servlet.MockMvc; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.put; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +/** + * HTTP-level coverage for the ownership guarantee: a caller can list, count, + * and mark-read only their own notifications, scoped by the email in their + * JWT — never by an id or query parameter a caller could substitute for + * someone else's. + */ +@SpringBootTest(properties = { + "booking.expiry-sweep-delay-ms=3600000", + "chain.events.poll-delay-ms=3600000", + "escrow.orchestration.submit-poll-delay-ms=3600000", + "escrow.orchestration.confirm-poll-delay-ms=3600000", + "escrow.reconciliation.poll-delay-ms=3600000" +}) +@AutoConfigureMockMvc +class NotificationControllerTest { + + @Autowired + private MockMvc mockMvc; + + @Autowired + private NotificationRepository notificationRepository; + + @Autowired + private JwtService jwtService; + + private String clientToken; + + @BeforeEach + void setUp() { + notificationRepository.deleteAll(); + clientToken = tokenFor("client@test.com"); + } + + private String tokenFor(String email) { + UserAccount account = new UserAccount(); + account.setId(1L); + account.setEmail(email); + account.setRole(Role.CLIENT); + return jwtService.generateAccessToken(account); + } + + private Notification seed(String recipientEmail, boolean read) { + Notification notification = new Notification(); + notification.setRecipientEmail(recipientEmail); + notification.setRecipientRole(Role.CLIENT); + notification.setType(NotificationType.APPOINTMENT_BOOKED); + notification.setTitle("Appointment booked"); + notification.setMessage("Your appointment was booked."); + notification.setRead(read); + return notificationRepository.save(notification); + } + + @Test + void listingWithoutATokenIsUnauthorized() throws Exception { + mockMvc.perform(get("/api/v1/notifications")) + .andExpect(status().isUnauthorized()) + .andExpect(content().contentTypeCompatibleWith(ProblemDetails.CONTENT_TYPE)); + } + + @Test + void listReturnsOnlyTheCallersOwnNotifications() throws Exception { + seed("client@test.com", false); + seed("someone-else@test.com", false); + + mockMvc.perform(get("/api/v1/notifications") + .header("Authorization", "Bearer " + clientToken)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.content.length()").value(1)) + .andExpect(jsonPath("$.content[0].title").value("Appointment booked")) + .andExpect(jsonPath("$.page.totalElements").value(1)); + } + + @Test + void unreadCountCountsOnlyTheCallersUnread() throws Exception { + seed("client@test.com", false); + seed("client@test.com", true); + seed("someone-else@test.com", false); + + mockMvc.perform(get("/api/v1/notifications/unread-count") + .header("Authorization", "Bearer " + clientToken)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.unread").value(1)); + } + + @Test + void markAsReadOnAnotherUsersNotificationIsNotFound() throws Exception { + Notification notification = seed("someone-else@test.com", false); + + mockMvc.perform(put("/api/v1/notifications/" + notification.getNotificationId() + "/read") + .header("Authorization", "Bearer " + clientToken)) + .andExpect(status().isNotFound()) + .andExpect(content().contentTypeCompatibleWith(ProblemDetails.CONTENT_TYPE)); + } + + @Test + void markAsReadOnOwnNotificationSucceeds() throws Exception { + Notification notification = seed("client@test.com", false); + + mockMvc.perform(put("/api/v1/notifications/" + notification.getNotificationId() + "/read") + .header("Authorization", "Bearer " + clientToken)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.read").value(true)); + + assertThat(notificationRepository.findById(notification.getNotificationId()).orElseThrow().isRead()).isTrue(); + } + + @Test + void markAllAsReadOnlyTouchesTheCallersOwnNotifications() throws Exception { + seed("client@test.com", false); + seed("client@test.com", false); + seed("someone-else@test.com", false); + + mockMvc.perform(put("/api/v1/notifications/read-all") + .header("Authorization", "Bearer " + clientToken)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.length()").value(2)); + + assertThat(notificationRepository.countByRecipientEmailAndReadFalse("someone-else@test.com")).isEqualTo(1); + assertThat(notificationRepository.countByRecipientEmailAndReadFalse("client@test.com")).isEqualTo(0); + } +} diff --git a/backend-api/src/test/java/com/guildworkman/api/services/ClientServiceTest.java b/backend-api/src/test/java/com/guildworkman/api/services/ClientServiceTest.java index 5e3ea73..520490b 100644 --- a/backend-api/src/test/java/com/guildworkman/api/services/ClientServiceTest.java +++ b/backend-api/src/test/java/com/guildworkman/api/services/ClientServiceTest.java @@ -18,12 +18,14 @@ import com.guildworkman.api.dto.responses.SkilledWorkerRegistrationResponse; import com.guildworkman.api.dto.responses.ViewAllAppointmentsResponse; import com.guildworkman.api.services.ServiceUtils.ClientService; +import com.guildworkman.api.services.ServiceUtils.MailService; import com.guildworkman.api.services.ServiceUtils.SkilledWorkerService; import org.jetbrains.annotations.NotNull; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.mock.mockito.MockBean; import org.springframework.test.context.jdbc.Sql; import org.springframework.transaction.annotation.Transactional; @@ -51,6 +53,10 @@ public class ClientServiceTest { private SkilledWorkerService skilledWorkerService; @PersistenceContext private EntityManager entityManager; + // Booking an appointment now fans out a notification email; mocked so + // these tests never make a real call to the mail provider. + @MockBean + private MailService mailService; @BeforeEach public void setUp() { diff --git a/backend-api/src/test/java/com/guildworkman/api/services/implmentations/AppointmentServiceImplTest.java b/backend-api/src/test/java/com/guildworkman/api/services/implmentations/AppointmentServiceImplTest.java new file mode 100644 index 0000000..c38dc17 --- /dev/null +++ b/backend-api/src/test/java/com/guildworkman/api/services/implmentations/AppointmentServiceImplTest.java @@ -0,0 +1,215 @@ +package com.guildworkman.api.services.implmentations; + +import com.guildworkman.api.booking.service.SlotReservationBooker; +import com.guildworkman.api.booking.service.SlotReservationService; +import com.guildworkman.api.data.constants.AppointmentStatus; +import com.guildworkman.api.data.constants.Category; +import com.guildworkman.api.data.constants.NotificationType; +import com.guildworkman.api.data.models.Appointment; +import com.guildworkman.api.data.models.Client; +import com.guildworkman.api.data.models.SkilledWorker; +import com.guildworkman.api.data.repository.AppointmentRepository; +import com.guildworkman.api.data.repository.ClientRepository; +import com.guildworkman.api.data.repository.SkilledWorkerRepository; +import com.guildworkman.api.dto.requests.AcceptAppointmentRequest; +import com.guildworkman.api.dto.requests.BookAppointmentRequest; +import com.guildworkman.api.dto.requests.UpdateAppointmentRequest; +import com.guildworkman.api.exceptions.AppointmentNotFoundException; +import com.guildworkman.api.exceptions.UserNotFoundException; +import com.guildworkman.api.services.ServiceUtils.NotificationService; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.modelmapper.ModelMapper; + +import java.time.LocalDateTime; +import java.util.Optional; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; + +/** + * Verifies that every appointment lifecycle transition + * (booked/accepted/declined/updated/cancelled/deleted) produces exactly the + * right {@link NotificationType} — the wiring {@code AppointmentServiceImpl} + * is responsible for, as opposed to the notification content itself, which + * {@code NotificationServiceImplTest} covers. + */ +class AppointmentServiceImplTest { + + private AppointmentRepository appointmentRepository; + private SkilledWorkerRepository skilledWorkerRepository; + private ClientRepository clientRepository; + private SlotReservationBooker slotReservationBooker; + private SlotReservationService slotReservationService; + private NotificationService notificationService; + private AppointmentServiceImpl service; + + @BeforeEach + void setUp() { + appointmentRepository = mock(AppointmentRepository.class); + skilledWorkerRepository = mock(SkilledWorkerRepository.class); + clientRepository = mock(ClientRepository.class); + slotReservationBooker = mock(SlotReservationBooker.class); + slotReservationService = mock(SlotReservationService.class); + notificationService = mock(NotificationService.class); + + when(appointmentRepository.save(any())).thenAnswer(invocation -> invocation.getArgument(0)); + + service = new AppointmentServiceImpl(appointmentRepository, new ModelMapper(), + skilledWorkerRepository, clientRepository, slotReservationBooker, + slotReservationService, notificationService); + } + + private static BookAppointmentRequest bookingRequest(Long clientId) { + BookAppointmentRequest request = new BookAppointmentRequest(); + request.setClientId(clientId); + request.setScheduleTime(LocalDateTime.now().plusDays(1)); + request.setCategory(Category.PLUMBING); + return request; + } + + private static Client client(Long id) { + Client client = new Client(); + client.setId(id); + client.setEmail("client" + id + "@test.com"); + client.setFullName("Client " + id); + return client; + } + + private static SkilledWorker worker(Long id) { + SkilledWorker worker = new SkilledWorker(); + worker.setId(id); + worker.setEmail("worker" + id + "@test.com"); + worker.setFullName("Worker " + id); + return worker; + } + + private static Appointment existingAppointment(Long id, AppointmentStatus status) { + Appointment appointment = new Appointment(); + appointment.setId(id); + appointment.setStatus(status); + appointment.setScheduleTime(LocalDateTime.now().plusDays(1)); + appointment.setClient(client(100L)); + appointment.setSkilledWorker(worker(200L)); + return appointment; + } + + // --- bookAppointment ------------------------------------------------------- + + @Test + void bookAppointmentResolvesClientAndNotifiesBooked() { + BookAppointmentRequest request = bookingRequest(3L); + Client client = client(3L); + when(clientRepository.findById(3L)).thenReturn(Optional.of(client)); + + Appointment result = service.bookAppointment(request); + + assertThat(result.getClient()).isSameAs(client); + assertThat(result.getStatus()).isEqualTo(AppointmentStatus.SCHEDULED); + verify(notificationService).notifyAppointmentEvent(result, NotificationType.APPOINTMENT_BOOKED); + } + + @Test + void bookAppointmentThrowsWhenClientNotFound() { + BookAppointmentRequest request = bookingRequest(404L); + when(clientRepository.findById(404L)).thenReturn(Optional.empty()); + + assertThatThrownBy(() -> service.bookAppointment(request)).isInstanceOf(UserNotFoundException.class); + verifyNoInteractions(notificationService); + } + + // --- cancelAppointment ------------------------------------------------------- + + @Test + void cancelAppointmentMarksCancelledAndNotifies() { + Appointment appointment = existingAppointment(5L, AppointmentStatus.SCHEDULED); + when(appointmentRepository.findById(5L)).thenReturn(Optional.of(appointment)); + + service.cancelAppointment(5L); + + assertThat(appointment.getStatus()).isEqualTo(AppointmentStatus.CANCELLED); + verify(notificationService).notifyAppointmentEvent(appointment, NotificationType.APPOINTMENT_CANCELLED); + } + + @Test + void cancelAppointmentThrowsWhenNotFound() { + when(appointmentRepository.findById(999L)).thenReturn(Optional.empty()); + + assertThatThrownBy(() -> service.cancelAppointment(999L)).isInstanceOf(AppointmentNotFoundException.class); + verifyNoInteractions(notificationService); + } + + // --- updateAppointment ------------------------------------------------------- + + @Test + void updateAppointmentAcceptedNotifiesAccepted() { + Appointment appointment = existingAppointment(6L, AppointmentStatus.SCHEDULED); + when(appointmentRepository.findById(6L)).thenReturn(Optional.of(appointment)); + UpdateAppointmentRequest request = new UpdateAppointmentRequest(); + request.setStatus(AppointmentStatus.ACCEPTED); + + service.updateAppointment(6L, request); + + verify(notificationService).notifyAppointmentEvent(appointment, NotificationType.APPOINTMENT_ACCEPTED); + } + + @Test + void updateAppointmentDeclinedReleasesSlotAndNotifiesDeclined() { + Appointment appointment = existingAppointment(7L, AppointmentStatus.SCHEDULED); + when(appointmentRepository.findById(7L)).thenReturn(Optional.of(appointment)); + UpdateAppointmentRequest request = new UpdateAppointmentRequest(); + request.setStatus(AppointmentStatus.DECLINED); + + service.updateAppointment(7L, request); + + verify(slotReservationService).releaseForAppointment(7L); + verify(notificationService).notifyAppointmentEvent(appointment, NotificationType.APPOINTMENT_DECLINED); + } + + @Test + void updateAppointmentWithNoStatusDefaultsToUpdatedAndNotifies() { + Appointment appointment = existingAppointment(8L, AppointmentStatus.SCHEDULED); + when(appointmentRepository.findById(8L)).thenReturn(Optional.of(appointment)); + UpdateAppointmentRequest request = new UpdateAppointmentRequest(); + + service.updateAppointment(8L, request); + + assertThat(appointment.getStatus()).isEqualTo(AppointmentStatus.UPDATED); + verify(notificationService).notifyAppointmentEvent(appointment, NotificationType.APPOINTMENT_UPDATED); + } + + // --- deleteAppointment ------------------------------------------------------- + + @Test + void deleteAppointmentNotifiesDeleted() { + Appointment appointment = existingAppointment(9L, AppointmentStatus.SCHEDULED); + when(appointmentRepository.findById(9L)).thenReturn(Optional.of(appointment)); + + service.deleteAppointment(9L); + + verify(appointmentRepository).delete(appointment); + verify(notificationService).notifyAppointmentEvent(appointment, NotificationType.APPOINTMENT_DELETED); + } + + // --- acceptAppointment ------------------------------------------------------- + + @Test + void acceptAppointmentNotifiesAccepted() { + Appointment appointment = existingAppointment(10L, AppointmentStatus.SCHEDULED); + when(appointmentRepository.findById(10L)).thenReturn(Optional.of(appointment)); + AcceptAppointmentRequest request = new AcceptAppointmentRequest(); + request.setAppointmentId(10L); + request.setStatus(AppointmentStatus.ACCEPTED); + + service.acceptAppointment(request); + + assertThat(appointment.getStatus()).isEqualTo(AppointmentStatus.ACCEPTED); + verify(notificationService).notifyAppointmentEvent(appointment, NotificationType.APPOINTMENT_ACCEPTED); + } +} diff --git a/backend-api/src/test/java/com/guildworkman/api/services/implmentations/NotificationEmailDispatcherTest.java b/backend-api/src/test/java/com/guildworkman/api/services/implmentations/NotificationEmailDispatcherTest.java new file mode 100644 index 0000000..816a085 --- /dev/null +++ b/backend-api/src/test/java/com/guildworkman/api/services/implmentations/NotificationEmailDispatcherTest.java @@ -0,0 +1,92 @@ +package com.guildworkman.api.services.implmentations; + +import com.guildworkman.api.data.constants.NotificationEmailStatus; +import com.guildworkman.api.data.models.Notification; +import com.guildworkman.api.data.repository.NotificationRepository; +import com.guildworkman.api.dto.requests.SendMailRequest; +import com.guildworkman.api.services.ServiceUtils.MailService; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; + +import java.util.Optional; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * {@link NotificationEmailDispatcher} is the boundary that has to absorb a + * down or failing mail provider without letting it escape — see + * {@code NotificationServiceImpl#scheduleEmail}, which runs this after the + * appointment write it reports on has already committed. Runs {@code dispatch} + * synchronously (the {@code @Async} annotation only applies through Spring's + * proxy, not a directly-constructed instance), which is fine here: the + * behaviour under test is what happens to the notification row, not the + * threading. + */ +class NotificationEmailDispatcherTest { + + private MailService mailService; + private NotificationRepository notificationRepository; + private NotificationEmailDispatcher dispatcher; + + @BeforeEach + void setUp() { + mailService = mock(MailService.class); + notificationRepository = mock(NotificationRepository.class); + dispatcher = new NotificationEmailDispatcher(mailService, notificationRepository); + } + + private static Notification notification() { + Notification notification = new Notification(); + notification.setNotificationId(1L); + notification.setEmailStatus(NotificationEmailStatus.PENDING); + return notification; + } + + @Test + void aSuccessfulSendMarksTheNotificationSent() { + Notification notification = notification(); + when(notificationRepository.findById(1L)).thenReturn(Optional.of(notification)); + when(mailService.sendMail(any())).thenReturn("mail sent successfully"); + + dispatcher.dispatch(1L, "client@test.com", "Cara Client", "Appointment booked", "

hi

"); + + ArgumentCaptor captor = ArgumentCaptor.forClass(SendMailRequest.class); + verify(mailService).sendMail(captor.capture()); + assertThat(captor.getValue().getRecipientEmail()).isEqualTo("client@test.com"); + assertThat(captor.getValue().getRecipientName()).isEqualTo("Cara Client"); + assertThat(captor.getValue().getSubject()).isEqualTo("Appointment booked"); + assertThat(notification.getEmailStatus()).isEqualTo(NotificationEmailStatus.SENT); + verify(notificationRepository).save(notification); + } + + /** The mail-provider-down case: a thrown exception must never escape. */ + @Test + void aFailingMailProviderMarksTheNotificationFailedAndDoesNotThrow() { + Notification notification = notification(); + when(notificationRepository.findById(1L)).thenReturn(Optional.of(notification)); + when(mailService.sendMail(any())).thenThrow(new RuntimeException("mail provider is down")); + + assertThatCode(() -> dispatcher.dispatch(1L, "client@test.com", "Cara Client", "subj", "content")) + .doesNotThrowAnyException(); + + assertThat(notification.getEmailStatus()).isEqualTo(NotificationEmailStatus.FAILED); + verify(notificationRepository).save(notification); + } + + @Test + void aNotificationThatNoLongerExistsIsSilentlySkipped() { + when(notificationRepository.findById(1L)).thenReturn(Optional.empty()); + when(mailService.sendMail(any())).thenReturn("mail sent successfully"); + + assertThatCode(() -> dispatcher.dispatch(1L, "client@test.com", "Cara Client", "subj", "content")) + .doesNotThrowAnyException(); + + verify(notificationRepository, org.mockito.Mockito.never()).save(any()); + } +} diff --git a/backend-api/src/test/java/com/guildworkman/api/services/implmentations/NotificationEmailFanOutIntegrationTest.java b/backend-api/src/test/java/com/guildworkman/api/services/implmentations/NotificationEmailFanOutIntegrationTest.java new file mode 100644 index 0000000..9b62e33 --- /dev/null +++ b/backend-api/src/test/java/com/guildworkman/api/services/implmentations/NotificationEmailFanOutIntegrationTest.java @@ -0,0 +1,156 @@ +package com.guildworkman.api.services.implmentations; + +import com.guildworkman.api.data.constants.Category; +import com.guildworkman.api.data.constants.NotificationEmailStatus; +import com.guildworkman.api.data.models.Appointment; +import com.guildworkman.api.data.models.Client; +import com.guildworkman.api.data.models.Notification; +import com.guildworkman.api.data.models.SkilledWorker; +import com.guildworkman.api.data.repository.AppointmentRepository; +import com.guildworkman.api.data.repository.ClientRepository; +import com.guildworkman.api.data.repository.NotificationRepository; +import com.guildworkman.api.data.repository.SkilledWorkerRepository; +import com.guildworkman.api.dto.requests.BookAppointmentRequest; +import com.guildworkman.api.services.ServiceUtils.AppointmentService; +import com.guildworkman.api.services.ServiceUtils.MailService; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.mock.mockito.MockBean; + +import java.time.Duration; +import java.time.Instant; +import java.time.LocalDateTime; +import java.util.List; +import java.util.UUID; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.when; + +/** + * End-to-end proof of the guarantee the issue asks for: a down mail provider + * must never fail or roll back the appointment operation that triggered the + * notification. Unlike {@link NotificationEmailDispatcherTest} (which drives + * the dispatcher directly, synchronously, in isolation), this goes through + * the real {@link AppointmentService} bean, a real transaction, and the real + * {@code @Async} executor — the only way to actually exercise + * {@code NotificationServiceImpl#scheduleEmail}'s after-commit wiring and + * {@code AsyncConfig}'s executor together. + */ +@SpringBootTest(properties = { + "booking.expiry-sweep-delay-ms=3600000", + "chain.events.poll-delay-ms=3600000", + "escrow.orchestration.submit-poll-delay-ms=3600000", + "escrow.orchestration.confirm-poll-delay-ms=3600000", + "escrow.reconciliation.poll-delay-ms=3600000" +}) +class NotificationEmailFanOutIntegrationTest { + + @Autowired + private AppointmentService appointmentService; + + @Autowired + private AppointmentRepository appointmentRepository; + + @Autowired + private NotificationRepository notificationRepository; + + @Autowired + private ClientRepository clientRepository; + + @Autowired + private SkilledWorkerRepository skilledWorkerRepository; + + @MockBean + private MailService mailService; + + private Client client; + private SkilledWorker worker; + + @BeforeEach + void setUp() { + client = clientRepository.saveAndFlush(newClient()); + worker = skilledWorkerRepository.saveAndFlush(newWorker()); + } + + private Client newClient() { + Client c = new Client(); + c.setFullName("Fan-Out Test Client"); + c.setEmail("fanout-client-" + UUID.randomUUID() + "@example.com"); + c.setUsername("fanout-client-" + UUID.randomUUID()); + c.setPhoneNumber(UUID.randomUUID().toString()); + return c; + } + + private SkilledWorker newWorker() { + SkilledWorker w = new SkilledWorker(); + w.setFullName("Fan-Out Test Worker"); + w.setEmail("fanout-worker-" + UUID.randomUUID() + "@example.com"); + w.setUsername("fanout-worker-" + UUID.randomUUID()); + w.setPhoneNumber(UUID.randomUUID().toString()); + w.setCategory(Category.PLUMBING); + return w; + } + + @Test + void aFailingMailProviderStillLetsTheBookingCommitAndMarksTheEmailFailed() { + when(mailService.sendMail(any())).thenThrow(new RuntimeException("mail provider is down")); + + BookAppointmentRequest request = new BookAppointmentRequest(); + request.setClientId(client.getId()); + request.setSkilledWorkerId(worker.getId()); + request.setCategory(Category.PLUMBING); + request.setScheduleTime(LocalDateTime.now().plusDays(1)); + + // The booking call itself must succeed: a down mail provider is not + // this thread's problem, by the time it even knows about it. + Appointment appointment = appointmentService.bookAppointment(request); + + assertThat(appointment.getId()).isNotNull(); + assertThat(appointmentRepository.findById(appointment.getId())).isPresent(); + + List notifications = awaitNotificationsFor(appointment.getId(), 2); + + assertThat(notifications).hasSize(2); + // Both the client and the worker got a persisted notification even + // though the email behind each one failed. + assertThat(notifications) + .extracting(Notification::getRecipientEmail) + .containsExactlyInAnyOrder(client.getEmail(), worker.getEmail()); + assertThat(notifications) + .extracting(Notification::getEmailStatus) + .containsOnly(NotificationEmailStatus.FAILED); + } + + /** + * Email dispatch is {@code @Async} and deferred to after-commit, so the + * {@code FAILED} status lands on a background thread some (short) time + * after {@code bookAppointment} returns. No Awaitility on the classpath + * here, so this polls with a bounded, short timeout instead. + */ + private List awaitNotificationsFor(Long appointmentId, int expectedCount) { + Instant deadline = Instant.now().plus(Duration.ofSeconds(5)); + List notifications; + do { + notifications = notificationRepository.findAll().stream() + .filter(n -> appointmentId.equals(n.getAppointmentId())) + .toList(); + if (notifications.size() >= expectedCount + && notifications.stream().allMatch(n -> n.getEmailStatus() != NotificationEmailStatus.PENDING)) { + return notifications; + } + sleepQuietly(); + } while (Instant.now().isBefore(deadline)); + return notifications; + } + + private void sleepQuietly() { + try { + Thread.sleep(50); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } +} diff --git a/backend-api/src/test/java/com/guildworkman/api/services/implmentations/NotificationServiceImplTest.java b/backend-api/src/test/java/com/guildworkman/api/services/implmentations/NotificationServiceImplTest.java new file mode 100644 index 0000000..e708d17 --- /dev/null +++ b/backend-api/src/test/java/com/guildworkman/api/services/implmentations/NotificationServiceImplTest.java @@ -0,0 +1,240 @@ +package com.guildworkman.api.services.implmentations; + +import com.guildworkman.api.data.constants.AppointmentStatus; +import com.guildworkman.api.data.constants.Category; +import com.guildworkman.api.data.constants.NotificationType; +import com.guildworkman.api.data.constants.Role; +import com.guildworkman.api.data.models.Appointment; +import com.guildworkman.api.data.models.Client; +import com.guildworkman.api.data.models.Notification; +import com.guildworkman.api.data.models.SkilledWorker; +import com.guildworkman.api.data.repository.NotificationRepository; +import com.guildworkman.api.dto.responses.NotificationResponse; +import com.guildworkman.api.exceptions.NotificationNotFoundException; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; +import org.springframework.data.domain.Page; +import org.springframework.data.domain.PageImpl; +import org.springframework.data.domain.PageRequest; +import org.springframework.data.domain.Pageable; + +import java.time.LocalDateTime; +import java.util.List; +import java.util.Optional; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +class NotificationServiceImplTest { + + private NotificationRepository notificationRepository; + private NotificationEmailDispatcher emailDispatcher; + private NotificationServiceImpl service; + + @BeforeEach + void setUp() { + notificationRepository = mock(NotificationRepository.class); + emailDispatcher = mock(NotificationEmailDispatcher.class); + service = new NotificationServiceImpl(notificationRepository, emailDispatcher); + + // save() just returns whatever was passed in, with an id assigned, like a + // real JpaRepository would for a new row. + when(notificationRepository.save(any(Notification.class))).thenAnswer(invocation -> { + Notification notification = invocation.getArgument(0); + if (notification.getNotificationId() == null) { + notification.setNotificationId((long) (Math.random() * 1_000_000)); + } + return notification; + }); + } + + private static Client client() { + Client client = new Client(); + client.setId(1L); + client.setEmail("client@test.com"); + client.setFullName("Cara Client"); + return client; + } + + private static SkilledWorker worker() { + SkilledWorker worker = new SkilledWorker(); + worker.setId(2L); + worker.setEmail("worker@test.com"); + worker.setFullName("Wes Worker"); + return worker; + } + + private static Appointment appointment(Client client, SkilledWorker worker) { + Appointment appointment = new Appointment(); + appointment.setId(42L); + appointment.setStatus(AppointmentStatus.SCHEDULED); + appointment.setScheduleTime(LocalDateTime.now().plusDays(1)); + appointment.setCategory(Category.ELECTRICAL); + appointment.setClient(client); + appointment.setSkilledWorker(worker); + return appointment; + } + + // --- notifyAppointmentEvent: recipients ------------------------------------- + + @Test + void bookedNotifiesBothClientAndWorker() { + Appointment appointment = appointment(client(), worker()); + + service.notifyAppointmentEvent(appointment, NotificationType.APPOINTMENT_BOOKED); + + ArgumentCaptor captor = ArgumentCaptor.forClass(Notification.class); + verify(notificationRepository, times(2)).save(captor.capture()); + List saved = captor.getAllValues(); + assertThat(saved).extracting(Notification::getRecipientEmail) + .containsExactlyInAnyOrder("client@test.com", "worker@test.com"); + assertThat(saved).allMatch(n -> n.getType() == NotificationType.APPOINTMENT_BOOKED); + assertThat(saved).allMatch(n -> n.getAppointmentId().equals(42L)); + assertThat(saved).allMatch(n -> !n.isRead()); + } + + @Test + void acceptedNotifiesOnlyTheClient() { + Appointment appointment = appointment(client(), worker()); + + service.notifyAppointmentEvent(appointment, NotificationType.APPOINTMENT_ACCEPTED); + + ArgumentCaptor captor = ArgumentCaptor.forClass(Notification.class); + verify(notificationRepository, times(1)).save(captor.capture()); + assertThat(captor.getValue().getRecipientEmail()).isEqualTo("client@test.com"); + assertThat(captor.getValue().getRecipientRole()).isEqualTo(Role.CLIENT); + } + + @Test + void declinedNotifiesOnlyTheClient() { + Appointment appointment = appointment(client(), worker()); + + service.notifyAppointmentEvent(appointment, NotificationType.APPOINTMENT_DECLINED); + + verify(notificationRepository, times(1)).save(any()); + } + + @Test + void missingWorkerOnlyNotifiesTheClient() { + Appointment appointment = appointment(client(), null); + + service.notifyAppointmentEvent(appointment, NotificationType.APPOINTMENT_BOOKED); + + ArgumentCaptor captor = ArgumentCaptor.forClass(Notification.class); + verify(notificationRepository, times(1)).save(captor.capture()); + assertThat(captor.getValue().getRecipientEmail()).isEqualTo("client@test.com"); + } + + @Test + void everyNotificationSchedulesAFanOutEmail() { + Appointment appointment = appointment(client(), worker()); + + service.notifyAppointmentEvent(appointment, NotificationType.APPOINTMENT_UPDATED); + + // No transaction is active in this unit test, so the email dispatch runs + // immediately rather than deferring to an afterCommit callback. + verify(emailDispatcher).dispatch(anyLong(), eq("client@test.com"), eq("Cara Client"), anyString(), anyString()); + verify(emailDispatcher).dispatch(anyLong(), eq("worker@test.com"), eq("Wes Worker"), anyString(), anyString()); + } + + // --- pagination / read state ------------------------------------------------- + + @Test + void getNotificationsDelegatesToRepositoryAndMapsToResponses() { + Notification notification = new Notification(); + notification.setNotificationId(1L); + notification.setRecipientEmail("client@test.com"); + notification.setType(NotificationType.APPOINTMENT_BOOKED); + notification.setTitle("Appointment booked"); + notification.setMessage("hi"); + Pageable pageable = PageRequest.of(0, 20); + Page page = new PageImpl<>(List.of(notification), pageable, 1); + when(notificationRepository.findByRecipientEmail("client@test.com", pageable)).thenReturn(page); + + Page result = service.getNotifications("client@test.com", pageable); + + assertThat(result.getTotalElements()).isEqualTo(1); + assertThat(result.getContent().get(0).id()).isEqualTo(1L); + } + + @Test + void countUnreadDelegatesToRepository() { + when(notificationRepository.countByRecipientEmailAndReadFalse("client@test.com")).thenReturn(3L); + + assertThat(service.countUnread("client@test.com")).isEqualTo(3L); + } + + // --- ownership scoping -------------------------------------------------------- + + @Test + void markAsReadOnlyFindsTheCallersOwnNotification() { + Notification notification = new Notification(); + notification.setNotificationId(1L); + notification.setRecipientEmail("client@test.com"); + when(notificationRepository.findByNotificationIdAndRecipientEmail(1L, "client@test.com")) + .thenReturn(Optional.of(notification)); + + NotificationResponse response = service.markAsRead(1L, "client@test.com"); + + assertThat(response.read()).isTrue(); + assertThat(notification.isRead()).isTrue(); + assertThat(notification.getReadAt()).isNotNull(); + } + + @Test + void markAsReadThrowsForANotificationThatIsntTheCallers() { + // The repository query is scoped by recipientEmail, so a notification id + // that belongs to someone else simply doesn't match — same 404 as one + // that doesn't exist at all, so a caller can't distinguish "not mine" + // from "never existed". + when(notificationRepository.findByNotificationIdAndRecipientEmail(1L, "someone-else@test.com")) + .thenReturn(Optional.empty()); + + assertThatThrownBy(() -> service.markAsRead(1L, "someone-else@test.com")) + .isInstanceOf(NotificationNotFoundException.class); + } + + @Test + void markAsReadIsIdempotentForAnAlreadyReadNotification() { + Notification notification = new Notification(); + notification.setNotificationId(1L); + notification.setRecipientEmail("client@test.com"); + notification.setRead(true); + LocalDateTime firstReadAt = LocalDateTime.now().minusDays(1); + when(notificationRepository.findByNotificationIdAndRecipientEmail(1L, "client@test.com")) + .thenReturn(Optional.of(notification)); + + service.markAsRead(1L, "client@test.com"); + + // Already read: no redundant save. + verify(notificationRepository, never()).save(any()); + } + + @Test + void markAllAsReadOnlyTouchesTheCallersUnreadNotifications() { + Notification first = new Notification(); + first.setNotificationId(1L); + first.setRecipientEmail("client@test.com"); + Notification second = new Notification(); + second.setNotificationId(2L); + second.setRecipientEmail("client@test.com"); + when(notificationRepository.findByRecipientEmailAndReadFalse("client@test.com")) + .thenReturn(List.of(first, second)); + + List result = service.markAllAsRead("client@test.com"); + + assertThat(result).hasSize(2); + assertThat(result).allMatch(NotificationResponse::read); + verify(notificationRepository, times(2)).save(any()); + } +}