Skip to content

Commit

Permalink
Feature/1009 cleanup dezerializer (#1217)
Browse files Browse the repository at this point in the history
* introduce Dezerializer Helper

* add tests for DezerializerHelper

* use correct identifier for id

* extract maps to constants

* include conversion into method

* add helper methods

* use helper

* add negative tests for DeserializerHelper.java

* add deserializer package to allowed packages for businessservice calls

* get id from path as backup

* add null check in getpath

* add new interface and reimplement serialize logic

* refactor naming

* implement review feedback
  • Loading branch information
kcinay055679 authored Nov 28, 2024
1 parent 736aa37 commit 534288a
Show file tree
Hide file tree
Showing 15 changed files with 525 additions and 381 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/backend-test-action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,4 @@ jobs:
distribution: 'adopt'

- name: Use Maven to run unittests and integration tests
run: mvn clean verify -X
run: mvn clean verify
17 changes: 17 additions & 0 deletions backend/src/main/java/ch/puzzle/okr/Constants.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
package ch.puzzle.okr;

import ch.puzzle.okr.dto.checkin.*;
import ch.puzzle.okr.dto.keyresult.*;

import java.util.Map;

import static java.util.Map.entry;

public class Constants {
private Constants() {
}
Expand All @@ -19,4 +26,14 @@ private Constants() {
public static final String USER_TEAM = "UserTeam";

public static final String BACK_LOG_QUARTER_LABEL = "Backlog";
public static final String CHECK_IN_KEY_RESULT_ID_ATTRIBUTE_NAME = "keyResultId";
public static final String KEY_RESULT_TYPE_ATTRIBUTE_NAME = "keyResultType";

public static final Map<String, Class<? extends KeyResultDto>> KEY_RESULT_MAP = Map.ofEntries(
entry(KEY_RESULT_TYPE_METRIC, KeyResultMetricDto.class),
entry(KEY_RESULT_TYPE_ORDINAL, KeyResultOrdinalDto.class));

public static final Map<String, Class<? extends CheckInDto>> CHECK_IN_MAP = Map.ofEntries(
entry(KEY_RESULT_TYPE_METRIC, CheckInMetricDto.class),
entry(KEY_RESULT_TYPE_ORDINAL, CheckInOrdinalDto.class));
}
Original file line number Diff line number Diff line change
@@ -1,51 +1,39 @@
package ch.puzzle.okr.deserializer;

import ch.puzzle.okr.dto.checkin.CheckInDto;
import ch.puzzle.okr.dto.checkin.CheckInMetricDto;
import ch.puzzle.okr.dto.checkin.CheckInOrdinalDto;
import ch.puzzle.okr.models.keyresult.KeyResult;
import ch.puzzle.okr.dto.checkin.*;
import ch.puzzle.okr.service.business.KeyResultBusinessService;
import com.fasterxml.jackson.core.JacksonException;
import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.databind.DeserializationContext;
import com.fasterxml.jackson.databind.JsonDeserializer;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.node.ObjectNode;
import org.springframework.http.HttpStatus;
import com.fasterxml.jackson.databind.*;
import org.springframework.stereotype.Component;
import org.springframework.web.server.ResponseStatusException;

import java.io.IOException;

import static ch.puzzle.okr.Constants.KEY_RESULT_TYPE_METRIC;
import static ch.puzzle.okr.Constants.KEY_RESULT_TYPE_ORDINAL;
import static ch.puzzle.okr.Constants.*;

@Component
public class CheckInDeserializer extends JsonDeserializer<CheckInDto> {
public class CheckInDeserializer extends JsonDeserializer<CheckInDto> implements MetricOrdinalDeserializer {

private final DeserializerHelper deserializerHelper;
private final KeyResultBusinessService keyResultBusinessService;

public CheckInDeserializer(KeyResultBusinessService keyResultBusinessService) {
public CheckInDeserializer(DeserializerHelper deserializerHelper,
KeyResultBusinessService keyResultBusinessService) {
this.deserializerHelper = deserializerHelper;
this.keyResultBusinessService = keyResultBusinessService;
}

@Override
public CheckInDto deserialize(JsonParser jsonParser, DeserializationContext deserializationContext)
throws IOException, JacksonException {
ObjectMapper mapper = (ObjectMapper) jsonParser.getCodec();
ObjectNode root = mapper.readTree(jsonParser);
if (root.has("keyResultId")) {
KeyResult keyResultOfCheckIn = keyResultBusinessService.getEntityById(root.get("keyResultId").asLong());
if (KEY_RESULT_TYPE_METRIC.equals(keyResultOfCheckIn.getKeyResultType())) {
return mapper.readValue(root.toString(), CheckInMetricDto.class);
} else if (KEY_RESULT_TYPE_ORDINAL.equals(keyResultOfCheckIn.getKeyResultType())) {
return mapper.readValue(root.toString(), CheckInOrdinalDto.class);
} else {
throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "unsupported checkIn DTO to deserialize");
}
} else {
throw new ResponseStatusException(HttpStatus.BAD_REQUEST,
"missing keyResult ID to deserialize checkIn DTO");
throws IOException {
return deserializerHelper.deserializeMetricOrdinal(jsonParser, CHECK_IN_MAP, this);
}

@Override
public String getKeyResultType(JsonNode root) {
JsonNode keyResultIdNode = root.get(CHECK_IN_KEY_RESULT_ID_ATTRIBUTE_NAME);
if (keyResultIdNode == null) {
return null;
}
return keyResultBusinessService.getEntityById(keyResultIdNode.asLong()).getKeyResultType();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package ch.puzzle.okr.deserializer;

import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.node.ObjectNode;
import org.springframework.http.HttpStatus;
import org.springframework.stereotype.Component;
import org.springframework.web.server.ResponseStatusException;

import java.io.IOException;
import java.util.Map;

@Component
public class DeserializerHelper {

public <T> T deserializeMetricOrdinal(JsonParser jsonParser, Map<String, Class<? extends T>> validTypes,
MetricOrdinalDeserializer deserializer) throws IOException {
ObjectMapper mapper = (ObjectMapper) jsonParser.getCodec();
ObjectNode root = mapper.readTree(jsonParser);

String keyResultType = deserializer.getKeyResultType(root);
if (isKeyResultTypeInvalid(keyResultType, validTypes)) {
throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "unsupported entity DTO to deserialize");
}

Class<? extends T> targetClass = validTypes.get(keyResultType);
return mapper.readValue(root.toString(), targetClass);
}

private <T> boolean isKeyResultTypeInvalid(String type, Map<String, Class<? extends T>> validTypes) {
return type == null || !validTypes.containsKey(type);
}
}
Original file line number Diff line number Diff line change
@@ -1,35 +1,34 @@
package ch.puzzle.okr.deserializer;

import ch.puzzle.okr.dto.keyresult.KeyResultDto;
import ch.puzzle.okr.dto.keyresult.KeyResultMetricDto;
import ch.puzzle.okr.dto.keyresult.KeyResultOrdinalDto;
import com.fasterxml.jackson.core.JacksonException;
import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.databind.DeserializationContext;
import com.fasterxml.jackson.databind.JsonDeserializer;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.node.ObjectNode;
import org.springframework.http.HttpStatus;
import org.springframework.web.server.ResponseStatusException;
import com.fasterxml.jackson.databind.*;

import java.io.IOException;

import static ch.puzzle.okr.Constants.KEY_RESULT_TYPE_METRIC;
import static ch.puzzle.okr.Constants.KEY_RESULT_TYPE_ORDINAL;
import static ch.puzzle.okr.Constants.*;

public class KeyResultDeserializer extends JsonDeserializer<KeyResultDto> implements MetricOrdinalDeserializer {

private final DeserializerHelper deserializerHelper;

public KeyResultDeserializer(DeserializerHelper deserializerHelper) {
this.deserializerHelper = deserializerHelper;
}

public class KeyResultDeserializer extends JsonDeserializer<KeyResultDto> {
@Override
public KeyResultDto deserialize(JsonParser jsonParser, DeserializationContext deserializationContext)
throws IOException, JacksonException {
ObjectMapper mapper = (ObjectMapper) jsonParser.getCodec();
ObjectNode root = mapper.readTree(jsonParser);
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(keyResultAttribute)
&& root.get(keyResultAttribute).asText().equals(KEY_RESULT_TYPE_ORDINAL)) {
return mapper.readValue(root.toString(), KeyResultOrdinalDto.class);
throws IOException {

return deserializerHelper.deserializeMetricOrdinal(jsonParser, KEY_RESULT_MAP, this);
}

@Override
public String getKeyResultType(JsonNode root) {
JsonNode keyResultId = root.get(KEY_RESULT_TYPE_ATTRIBUTE_NAME);
if (keyResultId == null) {
return null;
}
throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "unsupported keyResult DTO to deserialize");
return keyResultId.asText();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package ch.puzzle.okr.deserializer;

import com.fasterxml.jackson.databind.JsonNode;

public interface MetricOrdinalDeserializer {
String getKeyResultType(JsonNode root);
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ void authorizationServiceAccessedByControllerOrAuthorization() {
void businessServiceAccessedByControllerOrAuthorizationServiceOrMapper() {
JavaClasses importedClasses = getMainSourceClasses();
ArchRule rule = classes().that().resideInAPackage("..service.business..").should().onlyBeAccessed()
.byAnyPackage("..controller..", "..authorization..", "..mapper..", "..business");
.byAnyPackage("..controller..", "..authorization..", "..mapper..", "..business", "..deserializer");

rule.check(importedClasses);
}
Expand Down Expand Up @@ -141,11 +141,13 @@ void serviceLayerCheck() {
.layer("PersistenceService").definedBy("..service.persistence..") //
.layer("Repository").definedBy("..repository..") //
.layer("Mapper").definedBy("..mapper..") //
.layer("Deserializer").definedBy("..deserializer..") //

.whereLayer("Controller").mayNotBeAccessedByAnyLayer() //
.whereLayer("AuthorizationService").mayOnlyBeAccessedByLayers("Controller") //
.whereLayer("BusinessService")
.mayOnlyBeAccessedByLayers("Controller", "AuthorizationService", "Mapper", "BusinessService") //
.mayOnlyBeAccessedByLayers("Controller", "AuthorizationService", "Mapper", "BusinessService",
"Deserializer") //
.whereLayer("ValidationService").mayOnlyBeAccessedByLayers("BusinessService") //
.whereLayer("PersistenceService")
.mayOnlyBeAccessedByLayers("BusinessService", "PersistenceService", "ValidationService") //
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package ch.puzzle.okr.controller;

import ch.puzzle.okr.deserializer.DeserializerHelper;
import ch.puzzle.okr.mapper.checkin.CheckInMapper;
import ch.puzzle.okr.models.checkin.Zone;
import ch.puzzle.okr.models.keyresult.KeyResultMetric;
Expand All @@ -17,6 +18,7 @@
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest;
import org.springframework.boot.test.mock.mockito.MockBean;
import org.springframework.boot.test.mock.mockito.SpyBean;
import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType;
import org.springframework.security.test.context.support.WithMockUser;
Expand Down Expand Up @@ -45,6 +47,8 @@ class CheckInControllerIT {
private CheckInMapper checkInMapper;
@MockBean
private KeyResultBusinessService keyResultBusinessService;
@SpyBean
DeserializerHelper deserializerHelper;

@BeforeEach
void setUp() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
package ch.puzzle.okr.controller;

import ch.puzzle.okr.deserializer.DeserializerHelper;
import ch.puzzle.okr.mapper.ActionMapper;
import ch.puzzle.okr.mapper.checkin.CheckInMapper;
import ch.puzzle.okr.mapper.keyresult.KeyResultMapper;
import ch.puzzle.okr.models.Action;
import ch.puzzle.okr.models.checkin.CheckIn;
import ch.puzzle.okr.models.keyresult.KeyResultWithActionList;
import ch.puzzle.okr.models.keyresult.*;
import ch.puzzle.okr.service.authorization.ActionAuthorizationService;
import ch.puzzle.okr.service.authorization.KeyResultAuthorizationService;
import ch.puzzle.okr.service.business.KeyResultBusinessService;
import ch.puzzle.okr.service.persistence.ObjectivePersistenceService;
import ch.puzzle.okr.service.persistence.UserPersistenceService;
import org.hamcrest.Matchers;
Expand All @@ -20,6 +22,7 @@
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest;
import org.springframework.boot.test.mock.mockito.MockBean;
import org.springframework.boot.test.mock.mockito.SpyBean;
import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType;
import org.springframework.security.test.context.support.WithMockUser;
Expand Down Expand Up @@ -61,6 +64,10 @@ class KeyResultControllerIT {
UserPersistenceService userPersistenceService;
@MockBean
ObjectivePersistenceService objectivePersistenceService;
@MockBean
private KeyResultBusinessService keyResultBusinessService;
@SpyBean
DeserializerHelper deserializerHelper;
@Autowired
private MockMvc mvc;

Expand Down Expand Up @@ -277,6 +284,8 @@ void createEntityWithEnumKeys() throws Exception {
void createEntityShouldThrowErrorWhenInvalidDto() throws Exception {
BDDMockito.given(keyResultMapper.toKeyResult(any()))
.willThrow(new ResponseStatusException(HttpStatus.NOT_FOUND, "Error"));
BDDMockito.given(keyResultBusinessService.getEntityById(anyLong()))
.willReturn(KeyResultOrdinal.Builder.builder().withId(1L).build());

mvc.perform(post(URL_BASE).content(CREATE_BODY_ORDINAL).contentType(MediaType.APPLICATION_JSON)
.with(SecurityMockMvcRequestPostProcessors.csrf()))
Expand All @@ -292,7 +301,9 @@ void shouldReturnUpdatedKeyResult() throws Exception {
BDDMockito.given(keyResultAuthorizationService.isImUsed(any(), any())).willReturn(false);
BDDMockito.given(keyResultAuthorizationService.updateEntities(anyLong(), any(), anyList()))
.willReturn(new KeyResultWithActionList(metricKeyResult, List.of()));
BDDMockito.given(actionAuthorizationService.getActionsByKeyResult(any())).willReturn(anyList());

BDDMockito.given(keyResultBusinessService.getEntityById(anyLong()))
.willReturn(KeyResultMetric.Builder.builder().withId(1L).build());

mvc.perform(put(URL_TO_KEY_RESULT_1).contentType(MediaType.APPLICATION_JSON)
.with(SecurityMockMvcRequestPostProcessors.csrf()).content(JSON))
Expand Down
Loading

0 comments on commit 534288a

Please sign in to comment.