Update dependency org.springframework.boot:spring-boot-starter-parent…

… to v3.4.0 (#1194) * Update dependency org.springframework.boot:spring-boot-starter-parent to v3.4.0 * improve security filter chain * Replace sout with logger * Reset id before recreating an entity, making it effectively a new entity * Remove old debug statements * Fix unit tests * Update alignment when key result changes type * Run formatter * Add missing mocks for unit test * Add tests for AlignmentValidationService * Add tests for KeyResult * Add tests for AlignmentBusinessService * Remove unused import * Add E2E test to check if keyResultType can not be changed after a checkin --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: peggimann <[email protected]> Co-authored-by: Giannin <[email protected]>
puzzle · Nov 29, 2024 · c146cfb · c146cfb
1 parent f999fa8
commit c146cfb
Show file tree

Hide file tree

Showing 15 changed files with 381 additions and 42 deletions.
diff --git a/backend/src/main/java/ch/puzzle/okr/SecurityConfig.java b/backend/src/main/java/ch/puzzle/okr/SecurityConfig.java
@@ -57,19 +57,13 @@ public SecurityFilterChain apiSecurityFilterChain(HttpSecurity http, @Value("${c
         setHeaders(http);
         http.addFilterAfter(new ForwardFilter(), BasicAuthenticationFilter.class);
         logger.debug("*** apiSecurityFilterChain reached");
+        setHeaders(http);
         return http.cors(Customizer.withDefaults())
                 .authorizeHttpRequests(e -> e.requestMatchers("/api/**").authenticated().anyRequest().permitAll())
                 .exceptionHandling(e -> e.authenticationEntryPoint(new HttpStatusEntryPoint(HttpStatus.UNAUTHORIZED)))
                 .oauth2ResourceServer(oauth2 -> oauth2.jwt(Customizer.withDefaults())).build();
     }
 
-    @Bean
-    @Order(2)
-    public SecurityFilterChain securityHeadersFilter(HttpSecurity http) throws Exception {
-        logger.debug("*** SecurityHeader reached");
-        return setHeaders(http).build();
-    }
-
     @Bean
     JWTProcessor<SecurityContext> jwtProcessor(JWTClaimsSetAwareJWSKeySelector<SecurityContext> keySelector) {
         ConfigurableJWTProcessor<SecurityContext> jwtProcessor = new DefaultJWTProcessor<>();
@@ -87,16 +81,17 @@ JwtDecoder jwtDecoder(JWTProcessor<SecurityContext> jwtProcessor, OAuth2TokenVal
     }
 
     private HttpSecurity setHeaders(HttpSecurity http) throws Exception {
-        return http.headers(headers -> headers
-                .contentSecurityPolicy(c -> c.policyDirectives(okrContentSecurityPolicy()))
-                .crossOriginEmbedderPolicy(c -> c.policy(REQUIRE_CORP))
-                .crossOriginOpenerPolicy(c -> c.policy(OPENER_SAME_ORIGIN))
-                .crossOriginResourcePolicy(c -> c.policy(RESOURCE_SAME_ORIGIN))
-                .addHeaderWriter(new StaticHeadersWriter("X-Permitted-Cross-Domain-Policies", "none"))
-                .frameOptions(HeadersConfigurer.FrameOptionsConfig::deny)
-                .xssProtection(c -> c.headerValue(ENABLED_MODE_BLOCK))
-                .httpStrictTransportSecurity(c -> c.includeSubDomains(true).maxAgeInSeconds(31536000))
-                .referrerPolicy(c -> c.policy(NO_REFERRER)).permissionsPolicy(c -> c.policy(okrPermissionPolicy())));
+        return http
+                .headers(headers -> headers.contentSecurityPolicy(c -> c.policyDirectives(okrContentSecurityPolicy()))
+                        .crossOriginEmbedderPolicy(c -> c.policy(REQUIRE_CORP))
+                        .crossOriginOpenerPolicy(c -> c.policy(OPENER_SAME_ORIGIN))
+                        .crossOriginResourcePolicy(c -> c.policy(RESOURCE_SAME_ORIGIN))
+                        .addHeaderWriter(new StaticHeadersWriter("X-Permitted-Cross-Domain-Policies", "none"))
+                        .frameOptions(HeadersConfigurer.FrameOptionsConfig::deny)
+                        .xssProtection(c -> c.headerValue(ENABLED_MODE_BLOCK))
+                        .httpStrictTransportSecurity(c -> c.includeSubDomains(true).maxAgeInSeconds(31536000))
+                        .referrerPolicy(c -> c.policy(NO_REFERRER))
+                        .permissionsPolicyHeader(c -> c.policy(okrPermissionPolicy())));
     }
 
     private String okrContentSecurityPolicy() {

diff --git a/backend/src/main/java/ch/puzzle/okr/models/keyresult/KeyResult.java b/backend/src/main/java/ch/puzzle/okr/models/keyresult/KeyResult.java
@@ -122,6 +122,10 @@ public String getKeyResultType() {
         return keyResultType;
     }
 
+    public void resetId() {
+        this.id = null;
+    }
+
     private void setKeyResultType(String keyResultType) {
         this.keyResultType = keyResultType;
     }

diff --git a/backend/src/main/java/ch/puzzle/okr/service/business/AlignmentBusinessService.java b/backend/src/main/java/ch/puzzle/okr/service/business/AlignmentBusinessService.java
@@ -0,0 +1,37 @@
+package ch.puzzle.okr.service.business;
+
+import ch.puzzle.okr.models.alignment.Alignment;
+import ch.puzzle.okr.models.alignment.KeyResultAlignment;
+import ch.puzzle.okr.models.keyresult.KeyResult;
+import ch.puzzle.okr.service.persistence.AlignmentPersistenceService;
+import ch.puzzle.okr.service.validation.AlignmentValidationService;
+import org.springframework.stereotype.Service;
+
+import java.util.List;
+
+@Service
+public class AlignmentBusinessService {
+    private final AlignmentPersistenceService alignmentPersistenceService;
+    private final AlignmentValidationService validation;
+
+    public AlignmentBusinessService(AlignmentPersistenceService alignmentPersistenceService,
+            AlignmentValidationService validation) {
+        this.alignmentPersistenceService = alignmentPersistenceService;
+        this.validation = validation;
+    }
+
+    public Alignment updateEntity(Long id, Alignment entity) {
+        validation.validateOnUpdate(id, entity);
+        return alignmentPersistenceService.save(entity);
+    }
+
+    public void updateKeyResultId(Long oldId, KeyResult newKeyResult) {
+        List<KeyResultAlignment> alignments = alignmentPersistenceService.findByKeyResultAlignmentId(oldId);
+
+        alignments.forEach(a -> {
+            a.setAlignmentTarget(newKeyResult);
+            this.updateEntity(a.getId(), a);
+        });
+
+    }
+}
diff --git a/backend/src/main/java/ch/puzzle/okr/service/business/KeyResultBusinessService.java b/backend/src/main/java/ch/puzzle/okr/service/business/KeyResultBusinessService.java
@@ -5,6 +5,7 @@
 import ch.puzzle.okr.models.checkin.CheckIn;
 import ch.puzzle.okr.models.keyresult.KeyResult;
 import ch.puzzle.okr.models.keyresult.KeyResultWithActionList;
+import ch.puzzle.okr.service.persistence.AlignmentPersistenceService;
 import ch.puzzle.okr.service.persistence.KeyResultPersistenceService;
 import ch.puzzle.okr.service.validation.KeyResultValidationService;
 import jakarta.transaction.Transactional;
@@ -23,15 +24,17 @@ public class KeyResultBusinessService implements BusinessServiceInterface<Long,
     private final KeyResultPersistenceService keyResultPersistenceService;
     private final CheckInBusinessService checkInBusinessService;
     private final ActionBusinessService actionBusinessService;
+    private final AlignmentBusinessService alignmentBusinessServices;
     private final KeyResultValidationService validator;
 
     public KeyResultBusinessService(KeyResultPersistenceService keyResultPersistenceService,
             KeyResultValidationService validator, CheckInBusinessService checkInBusinessService,
-            ActionBusinessService actionBusinessService) {
+            ActionBusinessService actionBusinessService, AlignmentBusinessService alignmentBusinessService) {
         this.keyResultPersistenceService = keyResultPersistenceService;
         this.checkInBusinessService = checkInBusinessService;
         this.actionBusinessService = actionBusinessService;
         this.validator = validator;
+        this.alignmentBusinessServices = alignmentBusinessService;
     }
 
     @Override
@@ -88,10 +91,13 @@ public KeyResultWithActionList updateEntities(Long id, KeyResult keyResult, List
     private KeyResult recreateEntity(Long id, KeyResult keyResult, List<Action> actionList) {
         actionBusinessService.deleteEntitiesByKeyResultId(id);
         KeyResult recreatedEntity = keyResultPersistenceService.recreateEntity(id, keyResult);
+
         actionList.forEach(action -> {
             action.resetId();
             action.setKeyResult(recreatedEntity);
         });
+        alignmentBusinessServices.updateKeyResultId(id, recreatedEntity);
+
         return recreatedEntity;
     }
 

diff --git a/backend/src/main/java/ch/puzzle/okr/service/persistence/KeyResultPersistenceService.java b/backend/src/main/java/ch/puzzle/okr/service/persistence/KeyResultPersistenceService.java
@@ -3,6 +3,8 @@
 import ch.puzzle.okr.models.keyresult.KeyResult;
 import ch.puzzle.okr.repository.KeyResultRepository;
 import jakarta.transaction.Transactional;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.springframework.stereotype.Service;
 
 import java.util.List;
@@ -11,6 +13,7 @@
 
 @Service
 public class KeyResultPersistenceService extends PersistenceBase<KeyResult, Long, KeyResultRepository> {
+    private static final Logger logger = LoggerFactory.getLogger(KeyResultPersistenceService.class);
 
     protected KeyResultPersistenceService(KeyResultRepository repository) {
         super(repository);
@@ -27,11 +30,11 @@ public List<KeyResult> getKeyResultsByObjective(Long objectiveId) {
 
     @Transactional
     public KeyResult recreateEntity(Long id, KeyResult keyResult) {
-        System.out.println(keyResult.toString());
-        System.out.println("*".repeat(30));
         // delete entity in order to prevent duplicates in case of changed keyResultType
         deleteById(id);
-        System.out.printf("reached delete entity with %d", id);
+
+        // reset id of key result, so it gets saved as a new entity
+        keyResult.resetId();
         return save(keyResult);
     }
 

diff --git a/backend/src/main/java/ch/puzzle/okr/service/validation/AlignmentValidationService.java b/backend/src/main/java/ch/puzzle/okr/service/validation/AlignmentValidationService.java
@@ -0,0 +1,28 @@
+package ch.puzzle.okr.service.validation;
+
+import ch.puzzle.okr.models.alignment.Alignment;
+import ch.puzzle.okr.repository.AlignmentRepository;
+import ch.puzzle.okr.service.persistence.AlignmentPersistenceService;
+import org.springframework.stereotype.Service;
+
+@Service
+public class AlignmentValidationService
+        extends ValidationBase<Alignment, Long, AlignmentRepository, AlignmentPersistenceService> {
+
+    AlignmentValidationService(AlignmentPersistenceService persistenceService) {
+        super(persistenceService);
+    }
+
+    @Override
+    public void validateOnCreate(Alignment model) {
+        throw new UnsupportedOperationException();
+    }
+
+    @Override
+    public void validateOnUpdate(Long id, Alignment model) {
+        throwExceptionWhenIdIsNull(id);
+        throwExceptionWhenIdIsNull(model.getId());
+        throwExceptionWhenIdHasChanged(id, model.getId());
+        validate(model);
+    }
+}
diff --git a/backend/src/test/java/ch/puzzle/okr/models/keyresult/KeyResultTest.java b/backend/src/test/java/ch/puzzle/okr/models/keyresult/KeyResultTest.java
@@ -0,0 +1,27 @@
+package ch.puzzle.okr.models.keyresult;
+
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.util.stream.Stream;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+class KeyResultTest {
+
+    @ParameterizedTest
+    @MethodSource("provideKeyResults")
+    void resetIdShouldSetIdToNull(KeyResult keyResult) {
+
+        keyResult.resetId();
+
+        assertNull(keyResult.getId());
+    }
+
+    private static Stream<Arguments> provideKeyResults() {
+        return Stream.of(Arguments.of(KeyResultMetric.Builder.builder().withId(1L).build()),
+                Arguments.of(KeyResultOrdinal.Builder.builder().withId(1L).build()));
+    }
+
+}
diff --git a/backend/src/test/java/ch/puzzle/okr/service/business/AlignmentBusinessServiceTest.java b/backend/src/test/java/ch/puzzle/okr/service/business/AlignmentBusinessServiceTest.java
@@ -0,0 +1,99 @@
+package ch.puzzle.okr.service.business;
+
+import ch.puzzle.okr.exception.OkrResponseStatusException;
+import ch.puzzle.okr.models.alignment.Alignment;
+import ch.puzzle.okr.models.alignment.KeyResultAlignment;
+import ch.puzzle.okr.models.keyresult.KeyResult;
+import ch.puzzle.okr.models.keyresult.KeyResultMetric;
+import ch.puzzle.okr.service.persistence.AlignmentPersistenceService;
+import ch.puzzle.okr.service.validation.AlignmentValidationService;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.ArgumentCaptor;
+import org.mockito.InjectMocks;
+import org.mockito.junit.jupiter.MockitoExtension;
+import org.springframework.http.HttpStatus;
+import org.springframework.test.context.bean.override.mockito.MockitoBean;
+
+import javax.swing.*;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import static org.junit.jupiter.api.Assertions.*;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.doThrow;
+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;
+
+@ExtendWith(MockitoExtension.class)
+class AlignmentBusinessServiceTest {
+    @MockitoBean
+    AlignmentPersistenceService alignmentPersistenceService = mock(AlignmentPersistenceService.class);
+    @MockitoBean
+    AlignmentValidationService alignmentValidationService = mock(AlignmentValidationService.class);
+
+    KeyResult keyResult;
+    List<KeyResultAlignment> alignments;
+
+    @InjectMocks
+    AlignmentBusinessService alignmentBusinessService;
+
+    @BeforeEach
+    void setUp() {
+        this.keyResult = KeyResultMetric.Builder.builder().withId(1L).withBaseline(10.0)
+                .withDescription("Awesome Keyresult").withStretchGoal(100.0).build();
+
+        this.alignments = new ArrayList<>();
+        this.alignments.add(KeyResultAlignment.Builder.builder().withId(12L).withTargetKeyResult(keyResult).build());
+        this.alignments.add(KeyResultAlignment.Builder.builder().withId(132L).withTargetKeyResult(keyResult).build());
+        this.alignments.add(KeyResultAlignment.Builder.builder().withId(9L).withTargetKeyResult(keyResult).build());
+    }
+
+    @Test
+    void updateEntityShouldThrowExceptionWhenValidationFails() {
+        doThrow(new OkrResponseStatusException(HttpStatus.BAD_REQUEST, "Error Message"))
+                .when(alignmentValidationService).validateOnUpdate(eq(1L), any(KeyResultAlignment.class));
+
+        assertThrows(OkrResponseStatusException.class,
+                () -> alignmentBusinessService.updateEntity(1L, new KeyResultAlignment()));
+    }
+
+    @Test
+    void updateEntityShouldSaveNewEntity() {
+        Alignment mockedAlignment = mock(Alignment.class);
+        when(alignmentPersistenceService.save(any(Alignment.class))).thenAnswer(i -> i.getArguments()[0]);
+
+        Alignment alignment = alignmentBusinessService.updateEntity(1L, mockedAlignment);
+
+        verify(alignmentPersistenceService, times(1)).save(mockedAlignment);
+        assertEquals(mockedAlignment, alignment);
+    }
+
+    @Test
+    void updateKeyResultIdShouldUpdateKeyResult() {
+        KeyResult mockedKeyresult = mock(KeyResult.class);
+        when(alignmentPersistenceService.findByKeyResultAlignmentId(1L)).thenReturn(this.alignments);
+
+        alignmentBusinessService.updateKeyResultId(1L, mockedKeyresult);
+
+        ArgumentCaptor<KeyResultAlignment> captor = ArgumentCaptor.forClass(KeyResultAlignment.class);
+        verify(alignmentPersistenceService, times(3)).save(captor.capture());
+        captor.getAllValues().forEach(c -> assertEquals(c.getAlignmentTarget(), mockedKeyresult));
+    }
+
+    @Test
+    void updateKeyResultIdShouldUpdateNothingIfNoKeyResultAreFound() {
+        KeyResult mockedKeyresult = mock(KeyResult.class);
+        when(alignmentPersistenceService.findByKeyResultAlignmentId(1L)).thenReturn(List.of());
+
+        alignmentBusinessService.updateKeyResultId(1L, mockedKeyresult);
+
+        verify(alignmentPersistenceService, never()).save(any());
+    }
+}
diff --git a/backend/src/test/java/ch/puzzle/okr/service/business/KeyResultBusinessServiceTest.java b/backend/src/test/java/ch/puzzle/okr/service/business/KeyResultBusinessServiceTest.java
@@ -42,6 +42,8 @@ class KeyResultBusinessServiceTest {
     KeyResultValidationService validator;
     @Mock
     ActionBusinessService actionBusinessService;
+    @Mock
+    AlignmentBusinessService alignmentBusinessService;
     @InjectMocks
     private KeyResultBusinessService keyResultBusinessService;
     List<KeyResult> keyResults;
@@ -154,6 +156,7 @@ void shouldEditMetricKeyResultWhenATypeChange() {
         verify(checkInBusinessService, times(1)).getCheckInsByKeyResultId(1L);
         verify(actionBusinessService, times(1)).deleteEntitiesByKeyResultId(1L);
         verify(actionBusinessService, times(1)).createEntities(actions);
+        verify(alignmentBusinessService, times(1)).updateKeyResultId(1L, newKeyresult);
         assertEquals(1L, newKeyresult.getId());
         assertEquals("Keyresult Metric update", newKeyresult.getTitle());
     }
@@ -172,6 +175,7 @@ void shouldEditOrdinalKeyResultWhenATypeChange() {
         verify(checkInBusinessService, times(1)).getCheckInsByKeyResultId(1L);
         verify(actionBusinessService, times(1)).deleteEntitiesByKeyResultId(1L);
         verify(actionBusinessService, times(1)).createEntities(actions);
+        verify(alignmentBusinessService, times(1)).updateKeyResultId(1L, newKeyresult);
         assertEquals(1L, newKeyresult.getId());
         assertEquals("Keyresult Ordinal update", newKeyresult.getTitle());
     }

diff --git a/backend/src/test/java/ch/puzzle/okr/service/persistence/CompletedPersistenceServiceIT.java b/backend/src/test/java/ch/puzzle/okr/service/persistence/CompletedPersistenceServiceIT.java
@@ -127,17 +127,21 @@ void deleteCompletedIdShouldDeleteExistingCompletedByObjectiveId() {
 
     @Test
     void deleteCompletedShouldThrowExceptionWhenCompletedNotFound() {
-        createdCompleted = completedPersistenceService.save(createCompleted(33L));
-        completedPersistenceService.deleteById(createdCompleted.getId());
+        long noExistentId = getNonExistentId();
 
-        Long completedId = createdCompleted.getId();
         OkrResponseStatusException exception = assertThrows(OkrResponseStatusException.class,
-                () -> completedPersistenceService.findById(completedId));
+                () -> completedPersistenceService.findById(noExistentId));
 
-        List<ErrorDto> expectedErrors = List.of(new ErrorDto("MODEL_WITH_ID_NOT_FOUND", List.of(COMPLETED, "200")));
+        List<ErrorDto> expectedErrors = List
+                .of(new ErrorDto("MODEL_WITH_ID_NOT_FOUND", List.of(COMPLETED, String.valueOf(noExistentId))));
 
         assertEquals(NOT_FOUND, exception.getStatusCode());
         assertThat(expectedErrors).hasSameElementsAs(exception.getErrors());
         assertTrue(TestHelper.getAllErrorKeys(expectedErrors).contains(exception.getReason()));
     }
+
+    private long getNonExistentId() {
+        long id = completedPersistenceService.findAll().stream().mapToLong(Completed::getId).max().orElse(10L);
+        return id + 1;
+    }
 }