Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix sonar code smells #488

Merged
merged 3 commits into from
Oct 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,11 @@ public KeyResultDto deserialize(JsonParser jsonParser, DeserializationContext de
throws IOException, JacksonException {
ObjectMapper mapper = (ObjectMapper) jsonParser.getCodec();
ObjectNode root = mapper.readTree(jsonParser);
if (root.has("keyResultType") && root.get("keyResultType").asText().equals(KEY_RESULT_TYPE_METRIC)) {
String keyResultAttribute = "keyResultType";
if (root.has(keyResultAttribute) && root.get(keyResultAttribute).asText().equals(KEY_RESULT_TYPE_METRIC)) {
return mapper.readValue(root.toString(), KeyResultMetricDto.class);
} else if (root.has("keyResultType") && root.get("keyResultType").asText().equals(KEY_RESULT_TYPE_ORDINAL)) {
} else if (root.has(keyResultAttribute)
&& root.get(keyResultAttribute).asText().equals(KEY_RESULT_TYPE_ORDINAL)) {
return mapper.readValue(root.toString(), KeyResultOrdinalDto.class);
}
throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "unsupported keyResult DTO to deserialize");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ public static final class Builder {
private String keyResultTitle;

public Builder() {
// This builder can be empty, so that it can get called
}

public static Builder builder() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ public abstract static class Builder<T> {
private LocalDateTime modifiedOn;
private String keyResultType;

public Builder(String keyResultType) {
protected Builder(String keyResultType) {
this.keyResultType = keyResultType;
}

Expand Down
4 changes: 0 additions & 4 deletions backend/src/test/java/ch/puzzle/okr/KeyResultTestHelpers.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@

import java.time.LocalDate;
import java.time.LocalDateTime;
import java.util.Arrays;
import java.util.List;

import static ch.puzzle.okr.Constants.KEY_RESULT_TYPE_METRIC;
import static ch.puzzle.okr.Constants.KEY_RESULT_TYPE_ORDINAL;
Expand Down Expand Up @@ -84,8 +82,6 @@ public class KeyResultTestHelpers {
public static final CheckIn checkIn2 = CheckInMetric.Builder.builder().withValue(12D).withId(4L)
.withKeyResult(metricKeyResult).withCreatedBy(user).withCreatedOn(LocalDateTime.MAX)
.withChangeInfo(CHANGE_INFO_2).withInitiatives(INITIATIVES_2).build();
public static final List<CheckIn> checkInList = Arrays.asList(checkIn1, checkIn2);

public static final CheckInDto checkInDto1 = new CheckInMetricDto(1L, CHANGE_INFO_1, INITIATIVES_1, 6,
metricKeyResult.getId(), LocalDateTime.MAX, LocalDateTime.MAX, 23D);
public static final CheckInDto checkInDto2 = new CheckInMetricDto(4L, CHANGE_INFO_2, INITIATIVES_2, 5,
Expand Down
8 changes: 6 additions & 2 deletions backend/src/test/java/ch/puzzle/okr/OverviewTestHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,12 @@
import java.util.List;

public class OverviewTestHelper {
public static final long quarterId = 1L;
public static final long teamId = 1L;
public static final long QUARTER_ID = 1L;
public static final long TEAM_ID = 1L;

public static final List<Long> teamIds = List.of(1L, 2L, 3L, 4L);

private OverviewTestHelper() {
throw new IllegalStateException("Utility class");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,28 +30,29 @@
@WithMockUser(value = "spring")
@ExtendWith(MockitoExtension.class)
@WebMvcTest(AlignmentController.class)
public class AlignmentControllerIT {
class AlignmentControllerIT {
@Autowired
private MockMvc mvc;
@MockBean
private AlignmentSelectionBusinessService alignmentSelectionBusinessService;
@SpyBean
private AlignmentSelectionMapper alignmentSelectionMapper;

static String alignmentObjectiveName = "Objective 5";
static List<AlignmentSelection> alignmentSelectionPuzzle = List.of(
AlignmentSelection.Builder.builder().withAlignmentSelectionId(AlignmentSelectionId.of(1L, 20L))
.withObjectiveTitle("Objective 1").withKeyResultTitle("KeyResult 20").build(),
AlignmentSelection.Builder.builder().withAlignmentSelectionId(AlignmentSelectionId.of(1L, 40L))
.withObjectiveTitle("Objective 1").withKeyResultTitle("KeyResult 40").build());
static List<AlignmentSelection> alignmentSelectionOKR = List.of(
AlignmentSelection.Builder.builder().withAlignmentSelectionId(AlignmentSelectionId.of(5L, 21L))
.withObjectiveTitle("Objective 5").withKeyResultTitle("KeyResult 21").build(),
.withObjectiveTitle(alignmentObjectiveName).withKeyResultTitle("KeyResult 21").build(),
AlignmentSelection.Builder.builder().withAlignmentSelectionId(AlignmentSelectionId.of(5L, 41L))
.withObjectiveTitle("Objective 5").withKeyResultTitle("KeyResult 41").build(),
.withObjectiveTitle(alignmentObjectiveName).withKeyResultTitle("KeyResult 41").build(),
AlignmentSelection.Builder.builder().withAlignmentSelectionId(AlignmentSelectionId.of(5L, 61L))
.withObjectiveTitle("Objective 5").withKeyResultTitle("KeyResult 61").build(),
.withObjectiveTitle(alignmentObjectiveName).withKeyResultTitle("KeyResult 61").build(),
AlignmentSelection.Builder.builder().withAlignmentSelectionId(AlignmentSelectionId.of(5L, 81L))
.withObjectiveTitle("Objective 5").withKeyResultTitle("KeyResult 81").build());
.withObjectiveTitle(alignmentObjectiveName).withKeyResultTitle("KeyResult 81").build());
static AlignmentSelection alignmentSelectionEmptyKeyResults = AlignmentSelection.Builder.builder()
.withAlignmentSelectionId(AlignmentSelectionId.of(8L, null)).withObjectiveTitle("Objective 8").build();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
@WithMockUser(value = "spring")
@ExtendWith(MockitoExtension.class)
@WebMvcTest(CacheController.class)
public class CacheControllerIT {
class CacheControllerIT {
@Autowired
private MockMvc mvc;
@MockBean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ void shouldCreateCheckInOrdinal() throws Exception {
}

@Test
void shouldThrowException_WhenKeyResultIdMissing() throws Exception {
void shouldThrowExceptionWhenKeyResultIdMissing() throws Exception {
BDDMockito.given(keyResultBusinessService.getKeyResultById(anyLong()))
.willReturn(KeyResultMetric.Builder.builder().withId(1L).build());
BDDMockito.given(checkInBusinessService.createCheckIn(any(), any())).willReturn(checkInOrdinal);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
@WithMockUser(value = "spring")
@ExtendWith(MockitoExtension.class)
@WebMvcTest(ClientConfigController.class)
public class ClientConfigControllerIT {
class ClientConfigControllerIT {
@Autowired
private MockMvc mvc;
@MockBean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class CompleteControllerIT {
.withObjective(Objective.Builder.builder().withId(3L).withTitle("Gute Lernende").build())
.withComment("Wir haben es gut geschafft").build();

public static final String createBodySuccessful = """
public static final String SUCCESSFUL_CREATE_BODY = """
{
"id":null,
"objectiveId":3,
Expand All @@ -56,7 +56,7 @@ class CompleteControllerIT {
void createSuccessfulCompleted() throws Exception {
BDDMockito.given(this.completedBusinessService.createCompleted((any()))).willReturn(successfulCompleted);

mvc.perform(post(baseUrl).content(createBodySuccessful).contentType(MediaType.APPLICATION_JSON)
mvc.perform(post(baseUrl).content(SUCCESSFUL_CREATE_BODY).contentType(MediaType.APPLICATION_JSON)
.with(SecurityMockMvcRequestPostProcessors.csrf()))
.andExpect(MockMvcResultMatchers.status().is2xxSuccessful()).andExpect(jsonPath(JSON_PATH_ID, Is.is(1)))
.andExpect(jsonPath("$.objective.id", Is.is(3)))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import ch.puzzle.okr.mapper.checkin.CheckInMapper;
import ch.puzzle.okr.mapper.keyresult.KeyResultMapper;
import ch.puzzle.okr.models.checkin.CheckIn;
import ch.puzzle.okr.service.business.KeyResultBusinessService;
import ch.puzzle.okr.service.persistence.ObjectivePersistenceService;
import ch.puzzle.okr.service.persistence.UserPersistenceService;
Expand All @@ -23,7 +24,9 @@
import org.springframework.test.web.servlet.result.MockMvcResultMatchers;
import org.springframework.web.server.ResponseStatusException;

import java.util.Arrays;
import java.util.Collections;
import java.util.List;

import static ch.puzzle.okr.Constants.KEY_RESULT_TYPE_METRIC;
import static ch.puzzle.okr.Constants.KEY_RESULT_TYPE_ORDINAL;
Expand Down Expand Up @@ -111,6 +114,7 @@ void shouldNotFindTheKeyResultWithGivenId() throws Exception {

@Test
void shouldReturnCheckInsFromKeyResult() throws Exception {
List<CheckIn> checkInList = Arrays.asList(checkIn1, checkIn2);
BDDMockito.given(this.keyResultBusinessService.getAllCheckInsByKeyResult(5)).willReturn(checkInList);
BDDMockito.given(this.checkInMapper.toDto(checkIn1)).willReturn(checkInDto1);
BDDMockito.given(this.checkInMapper.toDto(checkIn2)).willReturn(checkInDto2);
Expand Down Expand Up @@ -187,14 +191,14 @@ void createOrdinalKeyResult() throws Exception {
}

@Test
void shouldThrowException_WhenKeyResultTypeMissing() throws Exception {
void shouldThrowExceptionWhenKeyResultTypeMissing() throws Exception {
mvc.perform(post(URL_BASE).content(CREATE_BODY_KEY_RESULT_TYPE_MISSING).contentType(MediaType.APPLICATION_JSON)
.with(SecurityMockMvcRequestPostProcessors.csrf()))
.andExpect(MockMvcResultMatchers.status().is4xxClientError());
}

@Test
void shouldThrowException_WhenKeyResultTypeUnknown() throws Exception {
void shouldThrowExceptionWhenKeyResultTypeUnknown() throws Exception {
mvc.perform(post(URL_BASE).content(CREATE_BODY_KEY_RESULT_TYPE_UNKNOWN).contentType(MediaType.APPLICATION_JSON)
.with(SecurityMockMvcRequestPostProcessors.csrf()))
.andExpect(MockMvcResultMatchers.status().is4xxClientError());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
@WithMockUser(value = "spring")
@ExtendWith(MockitoExtension.class)
@WebMvcTest(OverviewController.class)
public class OverviewControllerIT {
class OverviewControllerIT {
@Autowired
private MockMvc mvc;
@MockBean
Expand Down Expand Up @@ -79,13 +79,13 @@ public class OverviewControllerIT {
.withCheckInValue(15.0).withConfidence(5).withCreatedOn(LocalDateTime.now()).build());

static Overview overviewKuchen = Overview.Builder.builder().withOverviewId(OverviewId.of(3L, 8L, 20L, 40L))
.withTeamName("Kuchen").withObjectiveTitle("Objective 8").withObjectiveState(ONGOING).withQuarterId(1L)
.withTeamName(TEAM_KUCHEN).withObjectiveTitle("Objective 8").withObjectiveState(ONGOING).withQuarterId(1L)
.withQuarterLabel(QUARTER_LABEL).withKeyResultTitle(DESCRIPTION).withKeyResultType(KEY_RESULT_TYPE_METRIC)
.withUnit(CHF).withBaseline(5.0).withStretchGoal(20.0).withCheckInValue(15.0).withConfidence(5)
.withCreatedOn(LocalDateTime.now()).build();

static Overview overviewFindus = Overview.Builder.builder().withOverviewId(OverviewId.of(4L, -1L, -1L, -1L))
.withTeamName("Findus").build();
static Overview simpleOverview = Overview.Builder.builder().withOverviewId(OverviewId.of(4L, -1L, -1L, -1L))
.withTeamName(TEAM_KUCHEN).build();

@BeforeEach
void setUp() {
Expand Down Expand Up @@ -144,12 +144,12 @@ void shouldReturnOnlyFilteredObjectivesByQuarterAndTeam() throws Exception {
@Test
void shouldReturnTeamWithEmptyObjectiveListWhenNoObjectiveInFilteredQuarter() throws Exception {
BDDMockito.given(overviewBusinessService.getOverviewByQuarterIdAndTeamIds(2L, List.of(4L)))
.willReturn(List.of(overviewFindus));
.willReturn(List.of(simpleOverview));

mvc.perform(get("/api/v2/overview?quarter=2&team=4").contentType(MediaType.APPLICATION_JSON))
.andExpect(MockMvcResultMatchers.status().isOk()).andExpect(jsonPath("$", Matchers.hasSize(1)))
.andExpect(jsonPath(JSON_PATH_TEAM_ID, Is.is(4)))
.andExpect(jsonPath(JSON_PATH_TEAM_NAME, Is.is("Findus")))
.andExpect(jsonPath(JSON_PATH_TEAM_NAME, Is.is(TEAM_KUCHEN)))
.andExpect(jsonPath("$[0].objectives.size()", Is.is(0)));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;

public class AlignmentSelectionMapperTest {
class AlignmentSelectionMapperTest {
private final AlignmentSelectionMapper alignmentSelectionMapper = new AlignmentSelectionMapper();

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
import static org.junit.jupiter.api.Assertions.*;
import static org.springframework.http.HttpStatus.BAD_REQUEST;

public class OverviewMapperTest {
class OverviewMapperTest {
private final OverviewMapper overviewMapper = new OverviewMapper();

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ private static User createUser() {
}

@Test
void emptyUsersCache_ShouldClearUserCache() {
void emptyUsersCacheShouldClearUserCache() {
createdUser = userPersistenceService.getOrCreateUser(createUser());
User userBeforeClearCache = cache.get(USERNAME, User.class);

Expand All @@ -60,7 +60,7 @@ void emptyUsersCache_ShouldClearUserCache() {
}

@Test
void emptyAllCaches_ShouldClearAllCaches() {
void emptyAllCachesShouldClearAllCaches() {
createdUser = userPersistenceService.getOrCreateUser(createUser());
User userBeforeClearCache = cache.get(USERNAME, User.class);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@
import static org.junit.jupiter.api.Assertions.assertEquals;

@SpringIntegrationTest
public class ClientConfigServiceIT {
class ClientConfigServiceIT {

@Autowired
private ClientConfigService clientConfigService;

@Test
void saveKeyResult_ShouldSaveNewKeyResult() {
void saveKeyResultShouldSaveNewKeyResult() {
Map<String, String> configMap = clientConfigService.getConfigBasedOnActiveEnv();

assertEquals("prod", configMap.get("activeProfile"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
import static org.mockito.Mockito.*;

@ExtendWith(MockitoExtension.class)
public class AlignmentSelectionBusinessServiceTest {
class AlignmentSelectionBusinessServiceTest {

@InjectMocks
AlignmentSelectionBusinessService alignmentSelectionBusinessService;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
import static org.junit.jupiter.api.Assertions.assertEquals;

@SpringIntegrationTest
public class KeyResultBusinessServiceIT {
class KeyResultBusinessServiceIT {

private static final String KEY_RESULT_UPDATED = "Updated Key Result";
private static final Jwt jwtToken = mockJwtToken("findus", "Findus", "Peterson", "[email protected]");
Expand Down Expand Up @@ -79,7 +79,7 @@ void tearDown() {
}

@Test
void updateKeyResult_shouldUpdateKeyResultWithSameTypeMetric() {
void updateKeyResultShouldUpdateKeyResultWithSameTypeMetric() {
createdKeyResult = keyResultBusinessService.createKeyResult(createKeyResultMetric(null), jwtToken);
createdKeyResult.setTitle(KEY_RESULT_UPDATED);

Expand All @@ -90,7 +90,7 @@ void updateKeyResult_shouldUpdateKeyResultWithSameTypeMetric() {
}

@Test
void updateKeyResult_shouldUpdateKeyResultWithSameTypeOrdinal() {
void updateKeyResultShouldUpdateKeyResultWithSameTypeOrdinal() {
createdKeyResult = keyResultBusinessService.createKeyResult(createKeyResultOrdinal(null), jwtToken);
createdKeyResult.setTitle(KEY_RESULT_UPDATED);

Expand All @@ -101,7 +101,7 @@ void updateKeyResult_shouldUpdateKeyResultWithSameTypeOrdinal() {
}

@Test
void updateKeyResult_shouldRecreateKeyResultMetric() {
void updateKeyResultShouldRecreateKeyResultMetric() {
KeyResult savedKeyResult = keyResultBusinessService.createKeyResult(createKeyResultOrdinal(null), jwtToken);
KeyResult updatedKeyResult = createKeyResultMetric(savedKeyResult.getId());

Expand All @@ -111,7 +111,7 @@ void updateKeyResult_shouldRecreateKeyResultMetric() {
}

@Test
void updateKeyResult_shouldRecreateKeyResultOrdinal() {
void updateKeyResultShouldRecreateKeyResultOrdinal() {
KeyResult savedKeyResult = keyResultBusinessService.createKeyResult(createKeyResultMetric(null), jwtToken);
KeyResult updatedKeyResult = createKeyResultOrdinal(savedKeyResult.getId());

Expand All @@ -121,7 +121,7 @@ void updateKeyResult_shouldRecreateKeyResultOrdinal() {
}

@Test
void updateKeyResult_shouldUpdateKeyResultWithDifferentTypeAndCheckInMetric() {
void updateKeyResultShouldUpdateKeyResultWithDifferentTypeAndCheckInMetric() {
KeyResult savedKeyResult = keyResultBusinessService.createKeyResult(createKeyResultOrdinal(null), jwtToken);
checkInBusinessService.createCheckIn(createCheckInOrdinal(savedKeyResult), jwtToken);

Expand All @@ -133,7 +133,7 @@ void updateKeyResult_shouldUpdateKeyResultWithDifferentTypeAndCheckInMetric() {
}

@Test
void updateKeyResult_shouldUpdateKeyResultWithDifferentTypeAndCheckInOrdinal() {
void updateKeyResultShouldUpdateKeyResultWithDifferentTypeAndCheckInOrdinal() {
KeyResult savedKeyResult = keyResultBusinessService.createKeyResult(createKeyResultMetric(null), jwtToken);
checkInBusinessService.createCheckIn(createCheckInMetric(savedKeyResult), jwtToken);

Expand Down
Loading
Loading