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 <alexander.dobler3@bosch.io>
2020-08-26 09:59:44 +02:00
parent 9f3ff40658
commit fe8569593e
6 changed files with 59 additions and 87 deletions
--- 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<String, String> 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<String, String> data,
+    public DdiConfigData(@JsonProperty(value = "data") final Map<String, String> data,
            @JsonProperty(value = "mode") final DdiUpdateMode mode) {
-        super(id, time, status);
        this.data = data;
        this.mode = mode;
    }
--- 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<String, String> 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);
+    }
 }
--- 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<String, String> 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<String, String> 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<String, String> 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<String, String> updatedAttributes = targetManagement.getControllerAttributes(controllerId);
--- 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());
--- 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<String, String> attributes, final String execution)
-            throws JSONException {
-        return configData(id, attributes, execution, null);
+    public static JSONObject configData(final Map<String, String> attributes) throws JSONException {
+        return configData(attributes, null);
    }

-    public static JSONObject configData(final String id, final Map<String, String> attributes, final String execution,
-            final String mode) throws JSONException {
+    public static JSONObject configData(final Map<String, String> 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);
        }
--- 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")