diff --git a/dev-analytics/src/main/java/com/juliashtal/devanalytics/ai/scheduler/MetricsSummaryScheduler.java b/dev-analytics/src/main/java/com/juliashtal/devanalytics/ai/scheduler/MetricsSummaryScheduler.java index 314bcf2..5c8d215 100644 --- a/dev-analytics/src/main/java/com/juliashtal/devanalytics/ai/scheduler/MetricsSummaryScheduler.java +++ b/dev-analytics/src/main/java/com/juliashtal/devanalytics/ai/scheduler/MetricsSummaryScheduler.java @@ -3,6 +3,7 @@ import com.juliashtal.devanalytics.ai.model.MetricsSummaryDto; import com.juliashtal.devanalytics.ai.service.MetricsAiService; import com.juliashtal.devanalytics.config.SystemClock; +import com.juliashtal.devanalytics.metrics.service.MetricWriteGate; import com.juliashtal.devanalytics.metrics.service.MetricsService; import com.juliashtal.devanalytics.notification.NotificationDispatchService; import com.juliashtal.devanalytics.user.repository.UserRepository; @@ -28,6 +29,7 @@ public class MetricsSummaryScheduler { private final MetricsAiService metricsAiService; private final NotificationDispatchService notificationDispatch; private final SystemClock systemClock; + private final MetricWriteGate writeGate; @Scheduled(cron = "0 0 8 * * MON", zone = "UTC") public void generateWeeklySummaries() { @@ -39,7 +41,13 @@ public void generateWeeklySummaries() { userRepository.findAll().forEach(user -> { try { // Compute the week before summarising it: the nightly job may not have covered this ISO week. - metricsService.calculateDailyMetrics(user.getId(), from, to); + // Only the refresh is gated — a summary over stored snapshots still beats no summary + // for a week, which is what skipping the whole run would cost at this cadence. + boolean refreshed = writeGate.runExclusively( + () -> metricsService.calculateDailyMetrics(user.getId(), from, to)); + if (!refreshed) { + log.info("Metric refresh skipped for userId={}; summarising stored snapshots", user.getId()); + } MetricsSummaryDto summary = metricsAiService.generateSummary(user, from, to, null); log.debug("Generated weekly summary for userId={}", user.getId()); diff --git a/dev-analytics/src/main/java/com/juliashtal/devanalytics/config/RestTemplateConfig.java b/dev-analytics/src/main/java/com/juliashtal/devanalytics/config/RestTemplateConfig.java index a502994..1e94cf0 100644 --- a/dev-analytics/src/main/java/com/juliashtal/devanalytics/config/RestTemplateConfig.java +++ b/dev-analytics/src/main/java/com/juliashtal/devanalytics/config/RestTemplateConfig.java @@ -1,22 +1,30 @@ package com.juliashtal.devanalytics.config; +import org.springframework.beans.factory.annotation.Value; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; -import org.springframework.http.converter.json.MappingJackson2HttpMessageConverter; +import org.springframework.http.client.SimpleClientHttpRequestFactory; import org.springframework.web.client.RestTemplate; /** * Provides the shared RestTemplate used for outbound HTTP calls. + * + *
Timeouts bound each hop because the default factory waits forever, and + * {@code JiraProjectService.discoverProjectsFromJira} runs on the request thread — an unreachable + * Jira host would otherwise hold a Tomcat worker until the client gave up. They bound a single + * request, not a paged loop over many.
*/ @Configuration public class RestTemplateConfig { @Bean - public RestTemplate restTemplate() { - RestTemplate restTemplate = new RestTemplate(); - restTemplate.getMessageConverters().add(new MappingJackson2HttpMessageConverter()); - return restTemplate; + public RestTemplate restTemplate( + @Value("${app.http.connect-timeout-ms}") int connectTimeoutMs, + @Value("${app.http.read-timeout-ms}") int readTimeoutMs) { + SimpleClientHttpRequestFactory factory = new SimpleClientHttpRequestFactory(); + factory.setConnectTimeout(connectTimeoutMs); + factory.setReadTimeout(readTimeoutMs); + return new RestTemplate(factory); } } - diff --git a/dev-analytics/src/main/java/com/juliashtal/devanalytics/config/SecurityConfig.java b/dev-analytics/src/main/java/com/juliashtal/devanalytics/config/SecurityConfig.java index 321b45f..a04be8f 100644 --- a/dev-analytics/src/main/java/com/juliashtal/devanalytics/config/SecurityConfig.java +++ b/dev-analytics/src/main/java/com/juliashtal/devanalytics/config/SecurityConfig.java @@ -4,6 +4,7 @@ import com.juliashtal.devanalytics.security.service.CustomUserDetailsService; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; import org.springframework.security.authentication.AuthenticationManager; import org.springframework.security.authentication.AuthenticationProvider; @@ -99,6 +100,10 @@ public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Excepti .authorizeHttpRequests(auth -> auth .requestMatchers(PUBLIC_ENDPOINTS).permitAll() .requestMatchers("/api/admin/**").hasRole("ADMIN") + // Ordered before the team rule below: the first matching pattern wins. Named + // in full rather than as a subtree — "me" is a literal where every sibling + // route takes a {teamId}, so a wildcard here would shadow all of them. + .requestMatchers(HttpMethod.GET, "/api/teams/me/memberships").authenticated() .requestMatchers("/api/teams/**").hasAnyRole("MANAGER", "ADMIN") .anyRequest().authenticated() ) diff --git a/dev-analytics/src/main/java/com/juliashtal/devanalytics/metrics/service/MetricBackfillScheduler.java b/dev-analytics/src/main/java/com/juliashtal/devanalytics/metrics/service/MetricBackfillScheduler.java index c667a85..68c6810 100644 --- a/dev-analytics/src/main/java/com/juliashtal/devanalytics/metrics/service/MetricBackfillScheduler.java +++ b/dev-analytics/src/main/java/com/juliashtal/devanalytics/metrics/service/MetricBackfillScheduler.java @@ -11,9 +11,9 @@ * Drives {@link MetricBackfillService} over every user once a night, filling days inside each * user's collected history that have never been calculated. * - *Separate from {@link MetricsScheduler}, which only computes yesterday. Runs at 03:00 UTC; - * ordering against that job is not a correctness requirement, because both write through the - * same {@link MetricsService#calculateDailyMetrics} upsert guard.
+ *Separate from {@link MetricsScheduler}, which only computes yesterday. Runs at 03:00 UTC and + * takes {@link MetricWriteGate} first: the upsert guard both jobs share reads before it writes, so + * it orders writes but does not make concurrent ones safe.
*/ @Component @RequiredArgsConstructor @@ -22,9 +22,17 @@ public class MetricBackfillScheduler { private final UserRepository userRepository; private final MetricBackfillService backfillService; + private final MetricWriteGate writeGate; @Scheduled(cron = "0 0 3 * * ?", zone = "UTC") public void backfillAll() { + boolean ran = writeGate.runExclusively(this::backfillAllUsers); + if (!ran) { + log.info("History backfill job skipped: another metric writer is running"); + } + } + + private void backfillAllUsers() { log.info("History backfill job started"); userRepository.findAll().forEach(user -> { diff --git a/dev-analytics/src/main/java/com/juliashtal/devanalytics/metrics/service/MetricWriteGate.java b/dev-analytics/src/main/java/com/juliashtal/devanalytics/metrics/service/MetricWriteGate.java new file mode 100644 index 0000000..ac74964 --- /dev/null +++ b/dev-analytics/src/main/java/com/juliashtal/devanalytics/metrics/service/MetricWriteGate.java @@ -0,0 +1,40 @@ +package com.juliashtal.devanalytics.metrics.service; + +import org.springframework.stereotype.Component; + +import java.util.concurrent.locks.ReentrantLock; + +/** + * Serialises the scheduled jobs that write {@code metric_snapshots}; request threads, the + * {@code collect-} pool and the attribution listener still write outside it. + * + *The table carries no unique key, so the guard in {@code MetricSnapshotWriter} is a + * read-then-write: two writers computing the same window both miss {@code findExisting} and both + * insert. A process-local lock narrows that to the scheduled writers; a unique index over the + * {@code findExisting} columns is what would close it for every caller.
+ */ +@Component +public class MetricWriteGate { + + private final ReentrantLock lock = new ReentrantLock(); + + /** + * Runs {@code job} only while no other writer holds the gate. + * + *Skips rather than queues: each caller recomputes from persisted state, so the next run + * covers whatever this one declined.
+ * + * @return false when the job was skipped because another writer was running + */ + public boolean runExclusively(Runnable job) { + if (!lock.tryLock()) { + return false; + } + try { + job.run(); + return true; + } finally { + lock.unlock(); + } + } +} diff --git a/dev-analytics/src/main/java/com/juliashtal/devanalytics/metrics/service/MetricsScheduler.java b/dev-analytics/src/main/java/com/juliashtal/devanalytics/metrics/service/MetricsScheduler.java index b6d350c..d2d132c 100644 --- a/dev-analytics/src/main/java/com/juliashtal/devanalytics/metrics/service/MetricsScheduler.java +++ b/dev-analytics/src/main/java/com/juliashtal/devanalytics/metrics/service/MetricsScheduler.java @@ -24,10 +24,18 @@ public class MetricsScheduler { private final MetricsService metricsService; private final UserRepository userRepository; private final SystemClock systemClock; + private final MetricWriteGate writeGate; /** Runs every day at 01:00 UTC. */ @Scheduled(cron = "0 0 1 * * ?", zone = "UTC") public void calculateYesterday() { + boolean ran = writeGate.runExclusively(this::calculateYesterdayForAllUsers); + if (!ran) { + log.info("Nightly metrics scheduler skipped: another metric writer is running"); + } + } + + private void calculateYesterdayForAllUsers() { LocalDate yesterday = systemClock.yesterday(); log.info("Nightly metrics scheduler started for {}", yesterday); diff --git a/dev-analytics/src/main/resources/application.yml b/dev-analytics/src/main/resources/application.yml index bffd35a..b63ea66 100644 --- a/dev-analytics/src/main/resources/application.yml +++ b/dev-analytics/src/main/resources/application.yml @@ -1,4 +1,10 @@ spring: + task: + scheduling: + pool: + # Enough for the 2-minute enrichment pass to overrun without delaying the three + # nightly crons, which fire an hour apart. The default of 1 queues them behind it. + size: ${SCHEDULING_POOL_SIZE:4} servlet: multipart: max-file-size: 2MB @@ -41,6 +47,10 @@ app: refresh-expiration: 604800000 # 7 days password-reset: expiration: 3600000 # 1 hour + http: + # Bounds one outbound hop on the shared RestTemplate (Jira discovery and collection). + connect-timeout-ms: ${HTTP_CONNECT_TIMEOUT_MS:5000} + read-timeout-ms: ${HTTP_READ_TIMEOUT_MS:30000} base-url: ${APP_BASE_URL:http://localhost:8080} frontend-url: ${APP_FRONTEND_URL:http://localhost:5173} encryption: diff --git a/dev-analytics/src/test/java/com/juliashtal/devanalytics/ai/MetricsSummarySchedulerTest.java b/dev-analytics/src/test/java/com/juliashtal/devanalytics/ai/MetricsSummarySchedulerTest.java index 5268d25..695e454 100644 --- a/dev-analytics/src/test/java/com/juliashtal/devanalytics/ai/MetricsSummarySchedulerTest.java +++ b/dev-analytics/src/test/java/com/juliashtal/devanalytics/ai/MetricsSummarySchedulerTest.java @@ -4,6 +4,7 @@ import com.juliashtal.devanalytics.ai.scheduler.MetricsSummaryScheduler; import com.juliashtal.devanalytics.ai.service.MetricsAiService; import com.juliashtal.devanalytics.config.SystemClock; +import com.juliashtal.devanalytics.metrics.service.MetricWriteGate; import com.juliashtal.devanalytics.metrics.service.MetricsService; import com.juliashtal.devanalytics.notification.NotificationDispatchService; import com.juliashtal.devanalytics.user.model.User; @@ -61,7 +62,7 @@ void restoreZone() { @BeforeEach void setUp() { scheduler = new MetricsSummaryScheduler(userRepository, metricsService, metricsAiService, - notificationDispatch, new SystemClock(Clock.fixed(FIXED, ZoneOffset.UTC))); + notificationDispatch, new SystemClock(Clock.fixed(FIXED, ZoneOffset.UTC)), new MetricWriteGate()); } @Test @@ -117,6 +118,27 @@ void generateWeeklySummaries_oneUsersCalculationFailing_doesNotStopTheRest() { verify(metricsAiService, org.mockito.Mockito.never()).generateSummary(eq(failing), any(), any(), any()); } + /** + * A weekly job that skipped entirely on contention would leave the user with no brief for + * seven days, so only the refresh is allowed to be declined. + */ + @Test + void generateWeeklySummaries_anotherWriterHoldsGate_stillSummarisesStoredSnapshots() { + User user = user(1L); + MetricWriteGate busyGate = org.mockito.Mockito.mock(MetricWriteGate.class); + when(busyGate.runExclusively(any())).thenReturn(false); + scheduler = new MetricsSummaryScheduler(userRepository, metricsService, metricsAiService, + notificationDispatch, new SystemClock(Clock.fixed(FIXED, ZoneOffset.UTC)), busyGate); + when(userRepository.findAll()).thenReturn(List.of(user)); + when(metricsAiService.generateSummary(any(), any(), any(), any())) + .thenReturn(MetricsSummaryDto.builder().headline("h").build()); + + scheduler.generateWeeklySummaries(); + + org.mockito.Mockito.verifyNoInteractions(metricsService); + verify(metricsAiService).generateSummary(user, FROM, TO, null); + } + /** * The job fires at 08:00 UTC, an hour at which Auckland has already entered the next day and * Los Angeles is still in the previous one; the summarised week must not move with either. diff --git a/dev-analytics/src/test/java/com/juliashtal/devanalytics/config/RestTemplateConfigTest.java b/dev-analytics/src/test/java/com/juliashtal/devanalytics/config/RestTemplateConfigTest.java new file mode 100644 index 0000000..6165ffa --- /dev/null +++ b/dev-analytics/src/test/java/com/juliashtal/devanalytics/config/RestTemplateConfigTest.java @@ -0,0 +1,38 @@ +package com.juliashtal.devanalytics.config; + +import org.junit.jupiter.api.Test; +import org.springframework.http.client.ClientHttpRequestFactory; +import org.springframework.http.client.SimpleClientHttpRequestFactory; +import org.springframework.test.util.ReflectionTestUtils; +import org.springframework.web.client.RestTemplate; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Pins that the configured timeouts reach the request factory, so the shared RestTemplate cannot + * wait forever on an unresponsive host. + * + *The factory exposes no getters, so its fields are the only reachable evidence; a rename + * there fails this test loudly rather than silently restoring the unlimited default.
+ */ +class RestTemplateConfigTest { + + @Test + void restTemplate_configuredTimeouts_reachTheRequestFactory() { + RestTemplate restTemplate = new RestTemplateConfig().restTemplate(1_500, 9_000); + + ClientHttpRequestFactory factory = restTemplate.getRequestFactory(); + assertThat(factory).isInstanceOf(SimpleClientHttpRequestFactory.class); + assertThat(ReflectionTestUtils.getField(factory, "connectTimeout")).isEqualTo(1_500); + assertThat(ReflectionTestUtils.getField(factory, "readTimeout")).isEqualTo(9_000); + } + + @Test + void restTemplate_applicationDefaults_areFiniteAndPositive() { + RestTemplate restTemplate = new RestTemplateConfig().restTemplate(5_000, 30_000); + + ClientHttpRequestFactory factory = restTemplate.getRequestFactory(); + assertThat((Integer) ReflectionTestUtils.getField(factory, "connectTimeout")).isPositive(); + assertThat((Integer) ReflectionTestUtils.getField(factory, "readTimeout")).isPositive(); + } +} diff --git a/dev-analytics/src/test/java/com/juliashtal/devanalytics/metrics/MetricBackfillSchedulerTest.java b/dev-analytics/src/test/java/com/juliashtal/devanalytics/metrics/MetricBackfillSchedulerTest.java index 9b31a67..ddc7088 100644 --- a/dev-analytics/src/test/java/com/juliashtal/devanalytics/metrics/MetricBackfillSchedulerTest.java +++ b/dev-analytics/src/test/java/com/juliashtal/devanalytics/metrics/MetricBackfillSchedulerTest.java @@ -2,6 +2,7 @@ import com.juliashtal.devanalytics.metrics.model.BackfillResult; import com.juliashtal.devanalytics.metrics.service.MetricBackfillScheduler; +import com.juliashtal.devanalytics.metrics.service.MetricWriteGate; import com.juliashtal.devanalytics.metrics.service.MetricBackfillService; import com.juliashtal.devanalytics.user.model.User; import com.juliashtal.devanalytics.user.repository.UserRepository; @@ -33,12 +34,23 @@ void backfillAll_everyUser_isBackfilled() { when(backfillService.backfillUser(anyLong())) .thenReturn(new BackfillResult(0, 0, LocalDate.now(), LocalDate.now())); - new MetricBackfillScheduler(userRepository, backfillService).backfillAll(); + new MetricBackfillScheduler(userRepository, backfillService, new MetricWriteGate()).backfillAll(); verify(backfillService).backfillUser(1L); verify(backfillService).backfillUser(2L); } + /** Without this, removing the gate from the scheduler leaves every other test here green. */ + @Test + void backfillAll_anotherWriterHoldsGate_backfillsNothing() { + MetricWriteGate busyGate = mock(MetricWriteGate.class); + when(busyGate.runExclusively(any())).thenReturn(false); + + new MetricBackfillScheduler(userRepository, backfillService, busyGate).backfillAll(); + + verifyNoInteractions(backfillService, userRepository); + } + @Test void backfillAll_oneUserFails_othersStillProcessed() { when(userRepository.findAll()) @@ -49,7 +61,7 @@ void backfillAll_oneUserFails_othersStillProcessed() { when(backfillService.backfillUser(3L)) .thenReturn(new BackfillResult(0, 0, LocalDate.now(), LocalDate.now())); - new MetricBackfillScheduler(userRepository, backfillService).backfillAll(); + new MetricBackfillScheduler(userRepository, backfillService, new MetricWriteGate()).backfillAll(); verify(backfillService).backfillUser(1L); verify(backfillService).backfillUser(3L); diff --git a/dev-analytics/src/test/java/com/juliashtal/devanalytics/metrics/MetricWriteGateTest.java b/dev-analytics/src/test/java/com/juliashtal/devanalytics/metrics/MetricWriteGateTest.java new file mode 100644 index 0000000..3e1407a --- /dev/null +++ b/dev-analytics/src/test/java/com/juliashtal/devanalytics/metrics/MetricWriteGateTest.java @@ -0,0 +1,72 @@ +package com.juliashtal.devanalytics.metrics; + +import com.juliashtal.devanalytics.metrics.service.MetricWriteGate; +import org.junit.jupiter.api.Test; + +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +/** + * Pins that only one metric writer runs at a time, and that a declined run does not execute. + * + *Contention is produced with a real second thread rather than a re-entrant call: the gate + * holds a {@link java.util.concurrent.locks.ReentrantLock}, so a same-thread attempt would be + * admitted and the test would pass without proving anything.
+ */ +class MetricWriteGateTest { + + @Test + void runExclusively_noContention_runsJobAndReturnsTrue() { + MetricWriteGate gate = new MetricWriteGate(); + AtomicBoolean ran = new AtomicBoolean(false); + + boolean result = gate.runExclusively(() -> ran.set(true)); + + assertThat(result).isTrue(); + assertThat(ran).isTrue(); + } + + @Test + void runExclusively_otherWriterHolding_skipsJobAndReturnsFalse() throws Exception { + MetricWriteGate gate = new MetricWriteGate(); + CountDownLatch holding = new CountDownLatch(1); + CountDownLatch release = new CountDownLatch(1); + AtomicBoolean secondJobRan = new AtomicBoolean(false); + + Thread holder = new Thread(() -> gate.runExclusively(() -> { + holding.countDown(); + try { + release.await(5, TimeUnit.SECONDS); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + })); + holder.start(); + assertThat(holding.await(5, TimeUnit.SECONDS)).isTrue(); + + boolean result = gate.runExclusively(() -> secondJobRan.set(true)); + + release.countDown(); + holder.join(5_000); + + assertThat(result).isFalse(); + assertThat(secondJobRan).isFalse(); + } + + @Test + void runExclusively_jobThrows_releasesGateForTheNextRun() { + MetricWriteGate gate = new MetricWriteGate(); + + assertThatThrownBy(() -> gate.runExclusively(() -> { + throw new IllegalStateException("boom"); + })).isInstanceOf(IllegalStateException.class); + + AtomicBoolean ran = new AtomicBoolean(false); + assertThat(gate.runExclusively(() -> ran.set(true))).isTrue(); + assertThat(ran).isTrue(); + } +} diff --git a/dev-analytics/src/test/java/com/juliashtal/devanalytics/metrics/MetricsSchedulerTest.java b/dev-analytics/src/test/java/com/juliashtal/devanalytics/metrics/MetricsSchedulerTest.java index 544ef6e..eda36e6 100644 --- a/dev-analytics/src/test/java/com/juliashtal/devanalytics/metrics/MetricsSchedulerTest.java +++ b/dev-analytics/src/test/java/com/juliashtal/devanalytics/metrics/MetricsSchedulerTest.java @@ -1,6 +1,7 @@ package com.juliashtal.devanalytics.metrics; import com.juliashtal.devanalytics.config.SystemClock; +import com.juliashtal.devanalytics.metrics.service.MetricWriteGate; import com.juliashtal.devanalytics.metrics.service.MetricsScheduler; import com.juliashtal.devanalytics.metrics.service.MetricsService; import com.juliashtal.devanalytics.user.model.User; @@ -48,7 +49,7 @@ void restoreZone() { private MetricsScheduler scheduler() { return new MetricsScheduler(metricsService, userRepository, - new SystemClock(Clock.fixed(FIXED, ZoneOffset.UTC))); + new SystemClock(Clock.fixed(FIXED, ZoneOffset.UTC)), new MetricWriteGate()); } private User userWithId(Long id) { @@ -110,6 +111,18 @@ void calculateYesterday_computesExactlyYesterdayToYesterday_neverAWiderRange() { assertThat(toCaptor.getValue()).isEqualTo(YESTERDAY); } + /** Without this, removing the gate from the scheduler leaves every other test here green. */ + @Test + void calculateYesterday_anotherWriterHoldsGate_computesNothing() { + MetricWriteGate busyGate = mock(MetricWriteGate.class); + when(busyGate.runExclusively(any())).thenReturn(false); + + new MetricsScheduler(metricsService, userRepository, + new SystemClock(Clock.fixed(FIXED, ZoneOffset.UTC)), busyGate).calculateYesterday(); + + verifyNoInteractions(metricsService, userRepository); + } + /** * At the fixed instant the two zones sit on opposite sides of the date line, so a server-zone * reading would produce 2026-03-14 in Auckland and 2026-03-13 in Los Angeles. diff --git a/dev-analytics/src/test/java/com/juliashtal/devanalytics/user/TeamMembershipsControllerTest.java b/dev-analytics/src/test/java/com/juliashtal/devanalytics/user/TeamMembershipsControllerTest.java index c0bc18a..e7db809 100644 --- a/dev-analytics/src/test/java/com/juliashtal/devanalytics/user/TeamMembershipsControllerTest.java +++ b/dev-analytics/src/test/java/com/juliashtal/devanalytics/user/TeamMembershipsControllerTest.java @@ -1,5 +1,7 @@ package com.juliashtal.devanalytics.user; +import com.juliashtal.devanalytics.config.SecurityConfig; +import com.juliashtal.devanalytics.security.JwtAuthFilter; import com.juliashtal.devanalytics.security.service.CustomUserDetailsService; import com.juliashtal.devanalytics.security.service.JwtService; import com.juliashtal.devanalytics.user.controller.TeamController; @@ -8,9 +10,10 @@ import com.juliashtal.devanalytics.user.service.UserService; 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.autoconfigure.web.servlet.WebMvcTest; import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.context.annotation.Import; +import org.springframework.http.MediaType; import org.springframework.security.test.context.support.WithMockUser; import org.springframework.test.web.servlet.MockMvc; @@ -18,18 +21,29 @@ import static org.mockito.Mockito.when; 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.jsonPath; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; +/** + * Pins that a member with no managing role can read their own team memberships while the rest of + * {@code /api/teams} stays manager-only. + * + *{@link SecurityConfig} is imported because the {@code authorizeHttpRequests} path rules are + * evaluated before any controller, and a slice without them cannot observe a rejection.
+ */ @WebMvcTest(TeamController.class) -@AutoConfigureMockMvc(addFilters = false) +@Import({SecurityConfig.class, JwtAuthFilter.class}) class TeamMembershipsControllerTest { @Autowired MockMvc mvc; + @MockBean TeamService teamService; - @MockBean UserService userService; + // Required by SecurityConfig / JwtAuthFilter when filters are active @MockBean JwtService jwtService; @MockBean CustomUserDetailsService customUserDetailsService; + // Required by ActivityInterceptor (HandlerInterceptor picked up by @WebMvcTest) + @MockBean UserService userService; @Test @WithMockUser(roles = "DEVELOPER") @@ -52,4 +66,45 @@ void getMyMemberships_noTeams_returnsEmptyList() throws Exception { .andExpect(status().isOk()) .andExpect(jsonPath("$").isEmpty()); } + + @Test + @WithMockUser(roles = "MANAGER") + void getMyMemberships_asManager_returns200() throws Exception { + when(teamService.getMyMemberships()).thenReturn(List.of()); + + mvc.perform(get("/api/teams/me/memberships")) + .andExpect(status().isOk()); + } + + @Test + void getMyMemberships_unauthenticated_returns401() throws Exception { + mvc.perform(get("/api/teams/me/memberships")) + .andExpect(status().isUnauthorized()); + } + + /** The self-scoped exemption must not widen to the manager-only endpoints beside it. */ + @Test + @WithMockUser(roles = "DEVELOPER") + void getMyTeams_asDeveloper_returns403() throws Exception { + mvc.perform(get("/api/teams")) + .andExpect(status().isForbidden()); + } + + /** + * Every sibling route binds {@code me} to a {@code Long} {@code teamId}, so a subtree + * exemption would hand these to the controller and let type conversion decide the outcome. + */ + @Test + @WithMockUser(roles = "DEVELOPER") + void renameTeamNamedMe_asDeveloper_returns403NotAConversionError() throws Exception { + mvc.perform(put("/api/teams/me").contentType(MediaType.APPLICATION_JSON).content("{}")) + .andExpect(status().isForbidden()); + } + + @Test + @WithMockUser(roles = "DEVELOPER") + void exportTeamNamedMe_asDeveloper_returns403() throws Exception { + mvc.perform(get("/api/teams/me/export").param("from", "2026-01-01").param("to", "2026-01-31")) + .andExpect(status().isForbidden()); + } }