From fe8569593eca8e8c18510dd903bf643f77dd0942 Mon Sep 17 00:00:00 2001 From: Alexander Dobler <50330299+dobleralex@users.noreply.github.com> Date: Wed, 26 Aug 2020 09:59:44 +0200 Subject: [PATCH] Fix remove unused status of data config (#988) * Removed unused status from data config endpoint * Removed unused paramter from test * Formating fixes * Fixed comments after review Signed-off-by: Alexander Dobler --- .../hawkbit/ddi/json/model/DdiConfigData.java | 15 ++---- .../ddi/json/model/DdiConfigDataTest.java | 48 +++++++++--------- .../ddi/rest/resource/DdiConfigDataTest.java | 50 +++++++++---------- .../rest/resource/DdiRootControllerTest.java | 3 +- .../hawkbit/rest/util/JsonBuilder.java | 13 ++--- .../RootControllerDocumentationTest.java | 17 +------ 6 files changed, 59 insertions(+), 87 deletions(-) diff --git a/hawkbit-rest/hawkbit-ddi-api/src/main/java/org/eclipse/hawkbit/ddi/json/model/DdiConfigData.java b/hawkbit-rest/hawkbit-ddi-api/src/main/java/org/eclipse/hawkbit/ddi/json/model/DdiConfigData.java index 5188e8441..030d81a61 100644 --- a/hawkbit-rest/hawkbit-ddi-api/src/main/java/org/eclipse/hawkbit/ddi/json/model/DdiConfigData.java +++ b/hawkbit-rest/hawkbit-ddi-api/src/main/java/org/eclipse/hawkbit/ddi/json/model/DdiConfigData.java @@ -20,7 +20,7 @@ import com.fasterxml.jackson.annotation.JsonProperty; * Feedback channel for ConfigData action. */ @JsonIgnoreProperties(ignoreUnknown = true) -public class DdiConfigData extends DdiActionFeedback { +public class DdiConfigData { @NotEmpty private final Map data; @@ -30,21 +30,14 @@ public class DdiConfigData extends DdiActionFeedback { /** * Constructor. * - * @param id - * of the actions the feedback is for - * @param time - * of the feedback - * @param status - * is the feedback itself * @param data * contains the attributes. + * @param mode + * defines the mode of the update (replace, merge, remove) */ @JsonCreator - public DdiConfigData(@JsonProperty(value = "id") final Long id, @JsonProperty(value = "time") final String time, - @JsonProperty(value = "status") final DdiStatus status, - @JsonProperty(value = "data") final Map data, + public DdiConfigData(@JsonProperty(value = "data") final Map data, @JsonProperty(value = "mode") final DdiUpdateMode mode) { - super(id, time, status); this.data = data; this.mode = mode; } diff --git a/hawkbit-rest/hawkbit-ddi-api/src/test/java/org/eclipse/hawkbit/ddi/json/model/DdiConfigDataTest.java b/hawkbit-rest/hawkbit-ddi-api/src/test/java/org/eclipse/hawkbit/ddi/json/model/DdiConfigDataTest.java index b005b626e..2acb8c412 100644 --- a/hawkbit-rest/hawkbit-ddi-api/src/test/java/org/eclipse/hawkbit/ddi/json/model/DdiConfigDataTest.java +++ b/hawkbit-rest/hawkbit-ddi-api/src/test/java/org/eclipse/hawkbit/ddi/json/model/DdiConfigDataTest.java @@ -37,44 +37,28 @@ public class DdiConfigDataTest { @Description("Verify the correct serialization and deserialization of the model") public void shouldSerializeAndDeserializeObject() throws IOException { // Setup - Long id = 123L; - String time = "20190809T121314"; - DdiStatus ddiStatus = new DdiStatus(DdiStatus.ExecutionStatus.CLOSED, - new DdiResult(DdiResult.FinalResult.SUCCESS, null), null); Map data = new HashMap<>(); data.put("test", "data"); - DdiConfigData ddiConfigData = new DdiConfigData(id, time, ddiStatus, data, DdiUpdateMode.REPLACE); + DdiConfigData ddiConfigData = new DdiConfigData(data, DdiUpdateMode.REPLACE); // Test String serializedDdiConfigData = mapper.writeValueAsString(ddiConfigData); DdiConfigData deserializedDdiConfigData = mapper.readValue(serializedDdiConfigData, DdiConfigData.class); - assertThat(serializedDdiConfigData).contains(id.toString(), time, ddiStatus.getExecution().getName(), - ddiStatus.getResult().getFinished().getName(), "test", "data"); - assertThat(deserializedDdiConfigData.getId()).isEqualTo(id); - assertThat(deserializedDdiConfigData.getTime()).isEqualTo(time); - assertThat(deserializedDdiConfigData.getStatus().getExecution()).isEqualTo(DdiStatus.ExecutionStatus.CLOSED); - assertThat(deserializedDdiConfigData.getStatus().getResult().getFinished()).isEqualTo( - DdiResult.FinalResult.SUCCESS); + assertThat(serializedDdiConfigData).contains("test", "data"); assertThat(deserializedDdiConfigData.getMode()).isEqualTo(DdiUpdateMode.REPLACE); } @Test - @Description("Verify the correct deserialization of a model with a additional unknown property") + @Description("Verify the correct deserialization of a model with an additional unknown property") public void shouldDeserializeObjectWithUnknownProperty() throws IOException { // Setup - String serializedDdiConfigData = "{\"id\":123,\"time\":\"20190809T121314\"," - + "\"status\":{\"execution\":\"closed\",\"result\":{\"finished\":\"success\",\"progress\":null}," - + "\"details\":[]},\"data\":{\"test\":\"data\"},\"mode\":\"replace\",\"unknownProperty\":\"test\"}"; + String serializedDdiConfigData = "{\"data\":{\"test\":\"data\"},\"mode\":\"replace\",\"unknownProperty\":\"test\"}"; // Test DdiConfigData ddiConfigData = mapper.readValue(serializedDdiConfigData, DdiConfigData.class); - assertThat(ddiConfigData.getId()).isEqualTo(123); - assertThat(ddiConfigData.getTime()).isEqualTo("20190809T121314"); - assertThat(ddiConfigData.getStatus().getExecution()).isEqualTo(DdiStatus.ExecutionStatus.CLOSED); - assertThat(ddiConfigData.getStatus().getResult().getFinished()).isEqualTo(DdiResult.FinalResult.SUCCESS); assertThat(ddiConfigData.getMode()).isEqualTo(DdiUpdateMode.REPLACE); } @@ -82,11 +66,29 @@ public class DdiConfigDataTest { @Description("Verify that deserialization fails for known properties with a wrong datatype") public void shouldFailForObjectWithWrongDataTypes() throws IOException { // Setup - String serializedDdiConfigData = "{\"id\":[123],\"time\":\"20190809T121314\"," - + "\"status\":{\"execution\":\"closed\",\"result\":{\"finished\":\"success\",\"progress\":null}," - + "\"details\":[]},\"data\":{\"test\":\"data\"},\"mode\":\"replace\",\"unknownProperty\":\"test\"}"; + String serializedDdiConfigData = "{\"data\":{\"test\":\"data\"},\"mode\":[\"replace\"],\"unknownProperty\":\"test\"}"; // Test mapper.readValue(serializedDdiConfigData, DdiConfigData.class); } + + @Test + @Description("Verify the correct deserialization of a model with removed unused status property") + public void shouldDeserializeObjectWithStatusProperty() throws IOException { + // We formerly falsely required a 'status' property object when using the + // configData endpoint. It was removed as a requirement from code and + // documentation, as it was unused. This test ensures we still behave correctly + // (and just ignore the 'status' property) if it is still provided by the + // client. + + // Setup + String serializedDdiConfigData = "{\"id\":123,\"time\":\"20190809T121314\"," + + "\"status\":{\"execution\":\"closed\",\"result\":{\"finished\":\"success\",\"progress\":null}," + + "\"details\":[]},\"data\":{\"test\":\"data\"},\"mode\":\"replace\"}"; + + // Test + DdiConfigData ddiConfigData = mapper.readValue(serializedDdiConfigData, DdiConfigData.class); + + assertThat(ddiConfigData.getMode()).isEqualTo(DdiUpdateMode.REPLACE); + } } \ No newline at end of file diff --git a/hawkbit-rest/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiConfigDataTest.java b/hawkbit-rest/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiConfigDataTest.java index 2abb328a3..15c2157e5 100644 --- a/hawkbit-rest/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiConfigDataTest.java +++ b/hawkbit-rest/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiConfigDataTest.java @@ -24,8 +24,8 @@ import java.util.Map; import org.eclipse.hawkbit.ddi.rest.api.DdiRestConstants; import org.eclipse.hawkbit.exception.SpServerError; -import org.eclipse.hawkbit.repository.exception.InvalidTargetAttributeException; import org.eclipse.hawkbit.repository.exception.AssignmentQuotaExceededException; +import org.eclipse.hawkbit.repository.exception.InvalidTargetAttributeException; import org.eclipse.hawkbit.repository.model.Target; import org.eclipse.hawkbit.rest.util.JsonBuilder; import org.eclipse.hawkbit.rest.util.MockMvcResultPrinter; @@ -64,7 +64,7 @@ public class DdiConfigDataTest extends AbstractDDiApiIntegrationTest { attributes.put(KEY_VALID, VALUE_VALID); mvc.perform(put("/{tenant}/controller/v1/4717/configData", tenantAware.getCurrentTenant()) - .content(jsonToCbor(JsonBuilder.configData("", attributes, "closed").toString())) + .content(jsonToCbor(JsonBuilder.configData(attributes).toString())) .contentType(DdiRestConstants.MEDIA_TYPE_CBOR)).andDo(MockMvcResultPrinter.print()) .andExpect(status().isOk()); assertThat(targetManagement.getControllerAttributes("4717")).isEqualTo(attributes); @@ -121,17 +121,15 @@ public class DdiConfigDataTest extends AbstractDDiApiIntegrationTest { attributes.put(KEY_VALID, VALUE_VALID); mvc.perform(put("/{tenant}/controller/v1/4717/configData", tenantAware.getCurrentTenant()) - .content(JsonBuilder.configData("", attributes, "closed").toString()) - .contentType(MediaType.APPLICATION_JSON)).andDo(MockMvcResultPrinter.print()) - .andExpect(status().isOk()); + .content(JsonBuilder.configData(attributes).toString()).contentType(MediaType.APPLICATION_JSON)) + .andDo(MockMvcResultPrinter.print()).andExpect(status().isOk()); assertThat(targetManagement.getControllerAttributes("4717")).isEqualTo(attributes); // update attributes.put("sdsds", "123412"); mvc.perform(put("/{tenant}/controller/v1/4717/configData", tenantAware.getCurrentTenant()) - .content(JsonBuilder.configData("", attributes, "closed").toString()) - .contentType(MediaType.APPLICATION_JSON)).andDo(MockMvcResultPrinter.print()) - .andExpect(status().isOk()); + .content(JsonBuilder.configData(attributes).toString()).contentType(MediaType.APPLICATION_JSON)) + .andDo(MockMvcResultPrinter.print()).andExpect(status().isOk()); assertThat(targetManagement.getControllerAttributes("4717")).isEqualTo(attributes); } @@ -147,14 +145,14 @@ public class DdiConfigDataTest extends AbstractDDiApiIntegrationTest { attributes.put("dsafsdf" + i, "sdsds" + i); } mvc.perform(put("/{tenant}/controller/v1/4717/configData", tenantAware.getCurrentTenant()) - .content(JsonBuilder.configData("", attributes, "closed").toString()) - .contentType(MediaType.APPLICATION_JSON)).andExpect(status().isOk()); + .content(JsonBuilder.configData(attributes).toString()).contentType(MediaType.APPLICATION_JSON)) + .andExpect(status().isOk()); attributes = new HashMap<>(); attributes.put("on too many", "sdsds"); mvc.perform(put("/{tenant}/controller/v1/4717/configData", tenantAware.getCurrentTenant()) - .content(JsonBuilder.configData("", attributes, "closed").toString()) - .contentType(MediaType.APPLICATION_JSON)).andExpect(status().isForbidden()) + .content(JsonBuilder.configData(attributes).toString()).contentType(MediaType.APPLICATION_JSON)) + .andExpect(status().isForbidden()) .andExpect(jsonPath("$.exceptionClass", equalTo(AssignmentQuotaExceededException.class.getName()))) .andExpect(jsonPath("$.errorCode", equalTo(SpServerError.SP_QUOTA_EXCEEDED.getKey()))); @@ -181,14 +179,13 @@ public class DdiConfigDataTest extends AbstractDDiApiIntegrationTest { final Map attributes = new HashMap<>(); attributes.put("dsafsdf", "sdsds"); mvc.perform(put("/{tenant}/controller/v1/4712/configData", tenantAware.getCurrentTenant()) - .content(JsonBuilder.configData("", attributes, "closed").toString()).contentType(MediaTypes.HAL_JSON)) + .content(JsonBuilder.configData(attributes).toString()).contentType(MediaTypes.HAL_JSON)) .andDo(MockMvcResultPrinter.print()).andExpect(status().isUnsupportedMediaType()); // non existing target mvc.perform(put("/{tenant}/controller/v1/456456/configData", tenantAware.getCurrentTenant()) - .content(JsonBuilder.configData("", attributes, "closed").toString()) - .contentType(MediaType.APPLICATION_JSON)).andDo(MockMvcResultPrinter.print()) - .andExpect(status().isNotFound()); + .content(JsonBuilder.configData(attributes).toString()).contentType(MediaType.APPLICATION_JSON)) + .andDo(MockMvcResultPrinter.print()).andExpect(status().isNotFound()); // bad body mvc.perform(put("/{tenant}/controller/v1/4712/configData", tenantAware.getCurrentTenant()) @@ -215,8 +212,8 @@ public class DdiConfigDataTest extends AbstractDDiApiIntegrationTest { final Map attributes = Collections.singletonMap(KEY_TOO_LONG, VALUE_VALID); mvc.perform(put(configDataPath, tenantAware.getCurrentTenant()) - .content(JsonBuilder.configData("", attributes, "closed").toString()) - .contentType(MediaType.APPLICATION_JSON)).andExpect(status().isBadRequest()) + .content(JsonBuilder.configData(attributes).toString()).contentType(MediaType.APPLICATION_JSON)) + .andExpect(status().isBadRequest()) .andExpect(jsonPath("$.exceptionClass", equalTo(InvalidTargetAttributeException.class.getName()))) .andExpect(jsonPath("$.errorCode", equalTo(SpServerError.SP_TARGET_ATTRIBUTES_INVALID.getKey()))); } @@ -227,8 +224,8 @@ public class DdiConfigDataTest extends AbstractDDiApiIntegrationTest { final Map attributes = Collections.singletonMap(KEY_VALID, VALUE_TOO_LONG); mvc.perform(put(configDataPath, tenantAware.getCurrentTenant()) - .content(JsonBuilder.configData("", attributes, "closed").toString()) - .contentType(MediaType.APPLICATION_JSON)).andExpect(status().isBadRequest()) + .content(JsonBuilder.configData(attributes).toString()).contentType(MediaType.APPLICATION_JSON)) + .andExpect(status().isBadRequest()) .andExpect(jsonPath("$.exceptionClass", equalTo(InvalidTargetAttributeException.class.getName()))) .andExpect(jsonPath("$.errorCode", equalTo(SpServerError.SP_TARGET_ATTRIBUTES_INVALID.getKey()))); } @@ -269,7 +266,7 @@ public class DdiConfigDataTest extends AbstractDDiApiIntegrationTest { // use an invalid update mode mvc.perform(put(configDataPath, tenantAware.getCurrentTenant()) - .content(JsonBuilder.configData("", attributes, "closed", "KJHGKJHGKJHG").toString()) + .content(JsonBuilder.configData(attributes, "KJHGKJHGKJHG").toString()) .contentType(MediaType.APPLICATION_JSON)).andDo(MockMvcResultPrinter.print()) .andExpect(status().isBadRequest()); } @@ -287,7 +284,7 @@ public class DdiConfigDataTest extends AbstractDDiApiIntegrationTest { removeAttributes.put("k3", "bar"); mvc.perform(put(configDataPath, tenantAware.getCurrentTenant()) - .content(JsonBuilder.configData("", removeAttributes, "closed", "remove").toString()) + .content(JsonBuilder.configData(removeAttributes, "remove").toString()) .contentType(MediaType.APPLICATION_JSON)).andDo(MockMvcResultPrinter.print()) .andExpect(status().isOk()); @@ -310,7 +307,7 @@ public class DdiConfigDataTest extends AbstractDDiApiIntegrationTest { mergeAttributes.put("k1", "v1_modified_again"); mergeAttributes.put("k4", "v4"); mvc.perform(put(configDataPath, tenantAware.getCurrentTenant()) - .content(JsonBuilder.configData("", mergeAttributes, "closed", "merge").toString()) + .content(JsonBuilder.configData(mergeAttributes, "merge").toString()) .contentType(MediaType.APPLICATION_JSON)).andDo(MockMvcResultPrinter.print()) .andExpect(status().isOk()); @@ -336,7 +333,7 @@ public class DdiConfigDataTest extends AbstractDDiApiIntegrationTest { replacementAttributes.put("k2", "v2"); replacementAttributes.put("k3", "v3"); mvc.perform(put(configDataPath, tenantAware.getCurrentTenant()) - .content(JsonBuilder.configData("", replacementAttributes, "closed", "replace").toString()) + .content(JsonBuilder.configData(replacementAttributes, "replace").toString()) .contentType(MediaType.APPLICATION_JSON)).andDo(MockMvcResultPrinter.print()) .andExpect(status().isOk()); @@ -360,9 +357,8 @@ public class DdiConfigDataTest extends AbstractDDiApiIntegrationTest { // set the initial attributes mvc.perform(put(configDataPath, tenantAware.getCurrentTenant()) - .content(JsonBuilder.configData("", attributes, "closed").toString()) - .contentType(MediaType.APPLICATION_JSON)).andDo(MockMvcResultPrinter.print()) - .andExpect(status().isOk()); + .content(JsonBuilder.configData(attributes).toString()).contentType(MediaType.APPLICATION_JSON)) + .andDo(MockMvcResultPrinter.print()).andExpect(status().isOk()); // verify the initial parameters final Map updatedAttributes = targetManagement.getControllerAttributes(controllerId); diff --git a/hawkbit-rest/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiRootControllerTest.java b/hawkbit-rest/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiRootControllerTest.java index 50666258e..68849dc2e 100644 --- a/hawkbit-rest/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiRootControllerTest.java +++ b/hawkbit-rest/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiRootControllerTest.java @@ -398,8 +398,7 @@ public class DdiRootControllerTest extends AbstractDDiApiIntegrationTest { assertThatAttributesUpdateIsRequested(savedTarget.getControllerId()); mvc.perform(put("/{tenant}/controller/v1/{controllerId}/configData", tenantAware.getCurrentTenant(), - savedTarget.getControllerId()) - .content(JsonBuilder.configData(savedTarget.getControllerId(), attributes, "closed").toString()) + savedTarget.getControllerId()).content(JsonBuilder.configData(attributes).toString()) .contentType(MediaType.APPLICATION_JSON)) .andExpect(status().isOk()); assertThatAttributesUpdateIsNotRequested(savedTarget.getControllerId()); diff --git a/hawkbit-rest/hawkbit-rest-core/src/test/java/org/eclipse/hawkbit/rest/util/JsonBuilder.java b/hawkbit-rest/hawkbit-rest-core/src/test/java/org/eclipse/hawkbit/rest/util/JsonBuilder.java index 743d492cc..5dc1581a6 100644 --- a/hawkbit-rest/hawkbit-rest-core/src/test/java/org/eclipse/hawkbit/rest/util/JsonBuilder.java +++ b/hawkbit-rest/hawkbit-rest-core/src/test/java/org/eclipse/hawkbit/rest/util/JsonBuilder.java @@ -575,13 +575,11 @@ public abstract class JsonBuilder { } - public static JSONObject configData(final String id, final Map attributes, final String execution) - throws JSONException { - return configData(id, attributes, execution, null); + public static JSONObject configData(final Map attributes) throws JSONException { + return configData(attributes, null); } - public static JSONObject configData(final String id, final Map attributes, final String execution, - final String mode) throws JSONException { + public static JSONObject configData(final Map attributes, final String mode) throws JSONException { final JSONObject data = new JSONObject(); attributes.entrySet().forEach(entry -> { @@ -592,10 +590,7 @@ public abstract class JsonBuilder { } }); - final JSONObject json = new JSONObject().put("id", id).put("time", "20140511T121314") - .put("status", new JSONObject().put("execution", execution) - .put("result", new JSONObject().put("finished", "success")).put("details", new JSONArray())) - .put("data", data); + final JSONObject json = new JSONObject().put("data", data); if (mode != null) { json.put("mode", mode); } diff --git a/hawkbit-rest/hawkbit-rest-docs/src/test/java/org/eclipse/hawkbit/rest/ddi/documentation/RootControllerDocumentationTest.java b/hawkbit-rest/hawkbit-rest-docs/src/test/java/org/eclipse/hawkbit/rest/ddi/documentation/RootControllerDocumentationTest.java index 8470d55f3..fba390a53 100644 --- a/hawkbit-rest/hawkbit-rest-docs/src/test/java/org/eclipse/hawkbit/rest/ddi/documentation/RootControllerDocumentationTest.java +++ b/hawkbit-rest/hawkbit-rest-docs/src/test/java/org/eclipse/hawkbit/rest/ddi/documentation/RootControllerDocumentationTest.java @@ -212,26 +212,13 @@ public class RootControllerDocumentationTest extends AbstractApiRestDocumentatio mockMvc.perform( put(DdiRestConstants.BASE_V1_REQUEST_MAPPING + "/{controllerId}/" + DdiRestConstants.CONFIG_DATA_ACTION, tenantAware.getCurrentTenant(), target.getControllerId()) - .content(JsonBuilder.configData("", attributes, "closed", "merge").toString()) + .content(JsonBuilder.configData(attributes, "merge").toString()) .contentType(MediaType.APPLICATION_JSON_UTF8)) .andDo(MockMvcResultPrinter.print()).andExpect(status().isOk()) .andDo(this.document.document( pathParameters(parameterWithName("tenant").description(ApiModelPropertiesGeneric.TENANT), parameterWithName("controllerId").description(DdiApiModelProperties.CONTROLLER_ID)), - requestFields(optionalRequestFieldWithPath("id").description(DdiApiModelProperties.ACTION_ID), - optionalRequestFieldWithPath("time").description(DdiApiModelProperties.TARGET_TIME), - requestFieldWithPath("status").description(DdiApiModelProperties.TARGET_STATUS), - requestFieldWithPath("status.execution") - .description(DdiApiModelProperties.TARGET_EXEC_STATUS).type("enum") - .attributes(key("value").value( - "['closed', 'proceeding', 'canceled','scheduled', 'rejected', 'resumed']")), - requestFieldWithPath("status.result") - .description(DdiApiModelProperties.TARGET_RESULT_VALUE), - requestFieldWithPath("status.result.finished") - .description(DdiApiModelProperties.TARGET_RESULT_FINISHED).type("enum") - .attributes(key("value").value("['success', 'failure', 'none']")), - optionalRequestFieldWithPath("status.details") - .description(DdiApiModelProperties.TARGET_RESULT_DETAILS), + requestFields( requestFieldWithPath("data").description(DdiApiModelProperties.TARGET_CONFIG_DATA), optionalRequestFieldWithPath("mode").description(DdiApiModelProperties.UPDATE_MODE) .type("enum")