Posted feedback without "result" cause NPE in DDI API (#399)

* Fix for the bug by using @Valid on member variables

- add junit test
- done some refactoring

Signed-off-by: Jonathan Philip Knoblauch <JonathanPhilip.Knoblauch@bosch-si.com>

* Refactoring of junit tests

Signed-off-by: Jonathan Philip Knoblauch <JonathanPhilip.Knoblauch@bosch-si.com>
This commit is contained in:
Jonathan Knoblauch
2017-01-11 11:23:48 +01:00
committed by Kai Zimmermann
parent 5707e5908a
commit 824ef2982f
6 changed files with 146 additions and 66 deletions

View File

@@ -8,6 +8,7 @@
*/
package org.eclipse.hawkbit.ddi.json.model;
import javax.validation.Valid;
import javax.validation.constraints.NotNull;
import com.fasterxml.jackson.annotation.JsonCreator;
@@ -40,6 +41,7 @@ public class DdiActionFeedback {
private final String time;
@NotNull
@Valid
private final DdiStatus status;
/**

View File

@@ -8,7 +8,8 @@
*/
package org.eclipse.hawkbit.ddi.json.model;
import org.hibernate.validator.constraints.NotEmpty;
import javax.validation.Valid;
import javax.validation.constraints.NotNull;
import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
@@ -20,7 +21,8 @@ import com.fasterxml.jackson.annotation.JsonValue;
*/
public class DdiResult {
@NotEmpty
@NotNull
@Valid
private final FinalResult finished;
private final DdiProgress progress;

View File

@@ -11,6 +11,7 @@ package org.eclipse.hawkbit.ddi.json.model;
import java.util.Collections;
import java.util.List;
import javax.validation.Valid;
import javax.validation.constraints.NotNull;
import com.fasterxml.jackson.annotation.JsonCreator;
@@ -23,9 +24,11 @@ import com.fasterxml.jackson.annotation.JsonValue;
public class DdiStatus {
@NotNull
@Valid
private final ExecutionStatus execution;
@NotNull
@Valid
private final DdiResult result;
private final List<String> details;
@@ -60,8 +63,8 @@ public class DdiStatus {
public List<String> getDetails() {
if (details == null) {
return Collections.emptyList();
}
}
return Collections.unmodifiableList(details);
}

View File

@@ -27,6 +27,11 @@ import java.util.List;
import java.util.concurrent.TimeUnit;
import org.apache.commons.lang3.RandomUtils;
import org.eclipse.hawkbit.repository.event.remote.TargetAssignDistributionSetEvent;
import org.eclipse.hawkbit.repository.event.remote.entity.ActionCreatedEvent;
import org.eclipse.hawkbit.repository.event.remote.entity.DistributionSetCreatedEvent;
import org.eclipse.hawkbit.repository.event.remote.entity.TargetCreatedEvent;
import org.eclipse.hawkbit.repository.event.remote.entity.TargetUpdatedEvent;
import org.eclipse.hawkbit.repository.model.Action;
import org.eclipse.hawkbit.repository.model.Action.ActionType;
import org.eclipse.hawkbit.repository.model.Action.Status;
@@ -37,6 +42,8 @@ import org.eclipse.hawkbit.repository.model.DistributionSetAssignmentResult;
import org.eclipse.hawkbit.repository.model.RepositoryModelConstants;
import org.eclipse.hawkbit.repository.model.Target;
import org.eclipse.hawkbit.repository.model.TargetUpdateStatus;
import org.eclipse.hawkbit.repository.test.matcher.Expect;
import org.eclipse.hawkbit.repository.test.matcher.ExpectEvents;
import org.eclipse.hawkbit.rest.util.JsonBuilder;
import org.eclipse.hawkbit.rest.util.MockMvcResultPrinter;
import org.fest.assertions.core.Condition;
@@ -62,7 +69,7 @@ public class DdiDeploymentBaseTest extends AbstractDDiApiIntegrationTest {
private static final String HTTP_LOCALHOST = "http://localhost:8080/";
@Test()
@Test
@Description("Ensures that artifacts are not found, when softare module does not exists.")
public void artifactsNotFound() throws Exception {
final Target target = testdataFactory.createTarget();
@@ -72,7 +79,7 @@ public class DdiDeploymentBaseTest extends AbstractDDiApiIntegrationTest {
.andDo(MockMvcResultPrinter.print()).andExpect(status().isNotFound());
}
@Test()
@Test
@Description("Ensures that artifacts are found, when software module exists.")
public void artifactsExists() throws Exception {
final Target target = testdataFactory.createTarget();
@@ -596,7 +603,6 @@ public class DdiDeploymentBaseTest extends AbstractDDiApiIntegrationTest {
// action1 done
long current = System.currentTimeMillis();
long lastModified = targetManagement.findTargetByControllerID("4712").getLastModifiedAt();
mvc.perform(post("/{tenant}/controller/v1/4712/deploymentBase/" + action1.getId() + "/feedback",
tenantAware.getCurrentTenant())
.content(JsonBuilder.deploymentActionFeedback(action1.getId().toString(), "closed"))
@@ -605,11 +611,6 @@ public class DdiDeploymentBaseTest extends AbstractDDiApiIntegrationTest {
myT = targetManagement.findTargetByControllerIDWithDetails("4712");
assertThat(myT.getTargetInfo().getLastTargetQuery()).isLessThanOrEqualTo(System.currentTimeMillis());
assertThat(myT.getTargetInfo().getLastTargetQuery()).isGreaterThanOrEqualTo(current);
// assertThat( myT.getLastModifiedAt() ).isEqualTo( lastModified );
final long timeDiff = Math
.abs(myT.getTargetInfo().getLastTargetQuery() - myT.getTargetInfo().getInstallationDate());
assertThat(myT.getTargetInfo().getUpdateStatus()).isEqualTo(TargetUpdateStatus.PENDING);
assertThat(deploymentManagement.findActiveActionsByTarget(myT)).hasSize(2);
assertThat(myT.getTargetInfo().getInstalledDistributionSet().getId()).isEqualTo(ds1.getId());
@@ -622,7 +623,6 @@ public class DdiDeploymentBaseTest extends AbstractDDiApiIntegrationTest {
// action2 done
current = System.currentTimeMillis();
lastModified = targetManagement.findTargetByControllerID("4712").getLastModifiedAt();
mvc.perform(post("/{tenant}/controller/v1/4712/deploymentBase/" + action2.getId() + "/feedback",
tenantAware.getCurrentTenant())
.content(JsonBuilder.deploymentActionFeedback(action2.getId().toString(), "closed"))
@@ -643,7 +643,6 @@ public class DdiDeploymentBaseTest extends AbstractDDiApiIntegrationTest {
// action3 done
current = System.currentTimeMillis();
lastModified = targetManagement.findTargetByControllerID("4712").getLastModifiedAt();
mvc.perform(post("/{tenant}/controller/v1/4712/deploymentBase/" + action3.getId() + "/feedback",
tenantAware.getCurrentTenant())
.content(JsonBuilder.deploymentActionFeedback(action3.getId().toString(), "closed"))
@@ -678,7 +677,6 @@ public class DdiDeploymentBaseTest extends AbstractDDiApiIntegrationTest {
final Action action = deploymentManagement.findActionsByDistributionSet(pageReq, ds).getContent().get(0);
long current = System.currentTimeMillis();
long lastModified = targetManagement.findTargetByControllerID("4712").getLastModifiedAt();
mvc.perform(post("/{tenant}/controller/v1/4712/deploymentBase/" + action.getId() + "/feedback",
tenantAware.getCurrentTenant())
.content(JsonBuilder.deploymentActionFeedback(action.getId().toString(), "closed", "failure",
@@ -710,7 +708,6 @@ public class DdiDeploymentBaseTest extends AbstractDDiApiIntegrationTest {
assignDistributionSet(ds, toAssign);
final Action action2 = deploymentManagement.findActiveActionsByTarget(myT).get(0);
current = System.currentTimeMillis();
lastModified = targetManagement.findTargetByControllerID("4712").getLastModifiedAt();
mvc.perform(post("/{tenant}/controller/v1/4712/deploymentBase/" + action2.getId() + "/feedback",
tenantAware.getCurrentTenant())
@@ -920,12 +917,6 @@ public class DdiDeploymentBaseTest extends AbstractDDiApiIntegrationTest {
savedTarget = assignDistributionSet(savedSet, toAssign).getAssignedEntity().iterator().next();
assignDistributionSet(savedSet2, toAssign2);
// wrong format
mvc.perform(post("/{tenant}/controller/v1/4712/deploymentBase/AAAA/feedback", tenantAware.getCurrentTenant())
.content(JsonBuilder.deploymentActionInProgressFeedback("AAAA")).contentType(MediaType.APPLICATION_JSON)
.accept(MediaType.APPLICATION_JSON)).andDo(MockMvcResultPrinter.print())
.andExpect(status().isBadRequest());
final Action updateAction = deploymentManagement.findActiveActionsByTarget(savedTarget).get(0);
// action exists but is not assigned to this target
@@ -947,6 +938,68 @@ public class DdiDeploymentBaseTest extends AbstractDDiApiIntegrationTest {
}
@Test
@Description("Ensures that an invalid id in feedback body returns a bad request.")
@ExpectEvents({ @Expect(type = TargetCreatedEvent.class, count = 1),
@Expect(type = DistributionSetCreatedEvent.class, count = 1),
@Expect(type = TargetAssignDistributionSetEvent.class, count = 1),
@Expect(type = ActionCreatedEvent.class, count = 1), @Expect(type = TargetUpdatedEvent.class, count = 1) })
public void invalidIdInFeedbackReturnsBadRequest() throws Exception {
final Target target = testdataFactory.createTarget("1080");
final DistributionSet ds = testdataFactory.createDistributionSet("");
assignDistributionSet(ds.getId(), "1080");
final Action action = deploymentManagement.findActionsByTarget(target).get(0);
mvc.perform(post("/{tenant}/controller/v1/1080/deploymentBase/" + action.getId() + "/feedback",
tenantAware.getCurrentTenant()).content(JsonBuilder.deploymentActionInProgressFeedback("AAAA"))
.contentType(MediaType.APPLICATION_JSON).accept(MediaType.APPLICATION_JSON))
.andDo(MockMvcResultPrinter.print()).andExpect(status().isBadRequest());
}
@Test
@Description("Ensures that a missing feedback result in feedback body returns a bad request.")
@ExpectEvents({ @Expect(type = TargetCreatedEvent.class, count = 1),
@Expect(type = DistributionSetCreatedEvent.class, count = 1),
@Expect(type = TargetAssignDistributionSetEvent.class, count = 1),
@Expect(type = ActionCreatedEvent.class, count = 1), @Expect(type = TargetUpdatedEvent.class, count = 1) })
public void missingResultAttributeInFeedbackReturnsBadRequest() throws Exception {
final Target target = testdataFactory.createTarget("1080");
final DistributionSet ds = testdataFactory.createDistributionSet("");
assignDistributionSet(ds.getId(), "1080");
final Action action = deploymentManagement.findActionsByTarget(target).get(0);
final String missingResultInFeedback = JsonBuilder.missingResultInFeedback(action.getId().toString(), "closed",
"test");
mvc.perform(post("/{tenant}/controller/v1/1080/deploymentBase/" + action.getId() + "/feedback",
tenantAware.getCurrentTenant()).content(missingResultInFeedback).contentType(MediaType.APPLICATION_JSON)
.accept(MediaType.APPLICATION_JSON))
.andDo(MockMvcResultPrinter.print()).andExpect(status().isBadRequest());
}
@Test
@Description("Ensures that a missing finished result in feedback body returns a bad request.")
@ExpectEvents({ @Expect(type = TargetCreatedEvent.class, count = 1),
@Expect(type = DistributionSetCreatedEvent.class, count = 1),
@Expect(type = TargetAssignDistributionSetEvent.class, count = 1),
@Expect(type = ActionCreatedEvent.class, count = 1), @Expect(type = TargetUpdatedEvent.class, count = 1) })
public void missingFinishedAttributeInFeedbackReturnsBadRequest() throws Exception {
final Target target = testdataFactory.createTarget("1080");
final DistributionSet ds = testdataFactory.createDistributionSet("");
assignDistributionSet(ds.getId(), "1080");
final Action action = deploymentManagement.findActionsByTarget(target).get(0);
final String missingFinishedResultInFeedback = JsonBuilder
.missingFinishedResultInFeedback(action.getId().toString(), "closed", "test");
mvc.perform(post("/{tenant}/controller/v1/1080/deploymentBase/" + action.getId() + "/feedback",
tenantAware.getCurrentTenant()).content(missingFinishedResultInFeedback)
.contentType(MediaType.APPLICATION_JSON).accept(MediaType.APPLICATION_JSON))
.andDo(MockMvcResultPrinter.print()).andExpect(status().isBadRequest());
}
private class ActionStatusCondition extends Condition<ActionStatus> {
private final Status status;

View File

@@ -507,6 +507,7 @@ public class MgmtTargetResourceTest extends AbstractManagementApiIntegrationTest
}
@Test
@Description("Ensures that the get request for a target works.")
public void getSingleTarget() throws Exception {
// create first a target which can be retrieved by rest interface
final String knownControllerId = "1";
@@ -530,10 +531,12 @@ public class MgmtTargetResourceTest extends AbstractManagementApiIntegrationTest
}
@Test
@Description("Ensures that target get request returns a not found if the target does not exits.")
public void getSingleTargetNoExistsResponseNotFound() throws Exception {
final String targetIdNotExists = "bubu";
// test
final String targetIdNotExists = "bubu";
// test
final MvcResult mvcResult = mvc
.perform(get(MgmtRestConstants.TARGET_V1_REQUEST_MAPPING + "/" + targetIdNotExists))
.andExpect(status().isNotFound()).andReturn();
@@ -545,6 +548,7 @@ public class MgmtTargetResourceTest extends AbstractManagementApiIntegrationTest
}
@Test
@Description("Ensures that get request for asigned distribution sets returns no count if no distribution set has been assigned.")
public void getAssignedDistributionSetOfTargetIsEmpty() throws Exception {
// create first a target which can be retrieved by rest interface
final String knownControllerId = "1";
@@ -552,13 +556,13 @@ public class MgmtTargetResourceTest extends AbstractManagementApiIntegrationTest
createSingleTarget(knownControllerId, knownName);
// test
mvc.perform(get(MgmtRestConstants.TARGET_V1_REQUEST_MAPPING + "/" + knownControllerId + "/assignedDS"))
.andExpect(status().isNoContent()).andExpect(content().string(""));
}
@Test
@Description("Ensures that the get request for asigned distribution sets works.")
public void getAssignedDistributionSetOfTarget() throws Exception {
// create first a target which can be retrieved by rest interface
final String knownControllerId = "1";
@@ -614,6 +618,7 @@ public class MgmtTargetResourceTest extends AbstractManagementApiIntegrationTest
}
@Test
@Description("Ensures that get request for installed distribution sets returns no count if no distribution set has been installed.")
public void getInstalledDistributionSetOfTargetIsEmpty() throws Exception {
// create first a target which can be retrieved by rest interface
final String knownControllerId = "1";
@@ -625,6 +630,7 @@ public class MgmtTargetResourceTest extends AbstractManagementApiIntegrationTest
}
@Test
@Description("Ensures that post request for creating a target with no payload returns a bad request.")
public void createTargetWithoutPayloadBadRequest() throws Exception {
final MvcResult mvcResult = mvc
@@ -641,6 +647,7 @@ public class MgmtTargetResourceTest extends AbstractManagementApiIntegrationTest
}
@Test
@Description("Ensures that post request for creating a target with invalid payload returns a bad request.")
public void createTargetWithBadPayloadBadRequest() throws Exception {
final String notJson = "abc";
@@ -695,6 +702,7 @@ public class MgmtTargetResourceTest extends AbstractManagementApiIntegrationTest
}
@Test
@Description("Ensures that a post request for creating multiple targets works.")
public void createTargetsListReturnsSuccessful() throws Exception {
final Target test1 = entityFactory.target().create().controllerId("id1").name("testname1")
.securityToken("token").address("amqp://test123/foobar").description("testid1").build();
@@ -754,6 +762,7 @@ public class MgmtTargetResourceTest extends AbstractManagementApiIntegrationTest
}
@Test
@Description("Ensures that a post request for creating one target within a list works.")
public void createTargetsSingleEntryListReturnsSuccessful() throws Exception {
final String knownName = "someName";
final String knownControllerId = "controllerId1";
@@ -773,6 +782,7 @@ public class MgmtTargetResourceTest extends AbstractManagementApiIntegrationTest
}
@Test
@Description("Ensures that a post request for creating the same target again leads to a conflict response.")
public void createTargetsSingleEntryListDoubleReturnConflict() throws Exception {
final String knownName = "someName";
final String knownControllerId = "controllerId1";
@@ -802,29 +812,10 @@ public class MgmtTargetResourceTest extends AbstractManagementApiIntegrationTest
}
@Test
public void createTargetsSingleEntryListWithAdditionalNotExistingTargetAttributeWillBeIgnored() throws Exception {
final String knownName = "someName";
final String knownControllerId = "controllerId1";
final String knownDescription = "someDescription";
final String createTargetsJson = getCreateTargetsListJsonStringWithAdditionalNotExistingAttribute(
knownControllerId, knownName, knownDescription);
mvc.perform(post(MgmtRestConstants.TARGET_V1_REQUEST_MAPPING).content(createTargetsJson)
.contentType(MediaType.APPLICATION_JSON)).andDo(MockMvcResultPrinter.print())
.andExpect(status().is2xxSuccessful());
final Slice<Target> findTargetsAll = targetManagement.findTargetsAll(new PageRequest(0, 100));
final Target target = findTargetsAll.getContent().get(0);
assertThat(targetManagement.countTargetsAll()).isEqualTo(1);
assertThat(target.getControllerId()).isEqualTo(knownControllerId);
assertThat(target.getName()).isEqualTo(knownName);
assertThat(target.getDescription()).isEqualTo(knownDescription);
}
@Test
@Description("Ensures that the get request for action of a target returns no actions if nothing has happened.")
public void getActionWithEmptyResult() throws Exception {
final String knownTargetId = "targetId";
final Target target = testdataFactory.createTarget(knownTargetId);
testdataFactory.createTarget(knownTargetId);
mvc.perform(get(MgmtRestConstants.TARGET_V1_REQUEST_MAPPING + "/" + knownTargetId + "/"
+ MgmtRestConstants.TARGET_V1_ACTIONS)).andDo(MockMvcResultPrinter.print()).andExpect(status().isOk())
@@ -833,6 +824,7 @@ public class MgmtTargetResourceTest extends AbstractManagementApiIntegrationTest
}
@Test
@Description("Ensures that the expected response is return when update was cancelled.")
public void getCancelAction() throws Exception {
final String knownTargetId = "targetId";
final List<Action> actions = generateTargetWithTwoUpdatesWithOneOverride(knownTargetId);
@@ -850,6 +842,7 @@ public class MgmtTargetResourceTest extends AbstractManagementApiIntegrationTest
}
@Test
@Description("Ensures that the expected response of geting actions of a target is returned.")
public void getMultipleActions() throws Exception {
final String knownTargetId = "targetId";
final List<Action> actions = generateTargetWithTwoUpdatesWithOneOverride(knownTargetId);
@@ -873,7 +866,7 @@ public class MgmtTargetResourceTest extends AbstractManagementApiIntegrationTest
}
@Test
@Description("Verfies that the API returns the status list with expected content.")
@Description("Verifies that the API returns the status list with expected content.")
public void getMultipleActionStatus() throws Exception {
final String knownTargetId = "targetId";
final Action action = generateTargetWithTwoUpdatesWithOneOverride(knownTargetId).get(0);
@@ -899,7 +892,7 @@ public class MgmtTargetResourceTest extends AbstractManagementApiIntegrationTest
}
@Test
@Description("Verfies that the API returns the status list with expected content sorted by reportedAt field.")
@Description("Verifies that the API returns the status list with expected content sorted by reportedAt field.")
public void getMultipleActionStatusSortedByReportedAt() throws Exception {
final String knownTargetId = "targetId";
final Action action = generateTargetWithTwoUpdatesWithOneOverride(knownTargetId).get(0);
@@ -942,7 +935,7 @@ public class MgmtTargetResourceTest extends AbstractManagementApiIntegrationTest
}
@Test
@Description("Verfies that the API returns the status list with expected content split into two pages.")
@Description("Verifies that the API returns the status list with expected content split into two pages.")
public void getMultipleActionStatusWithPagingLimitRequestParameter() throws Exception {
final String knownTargetId = "targetId";
@@ -980,6 +973,7 @@ public class MgmtTargetResourceTest extends AbstractManagementApiIntegrationTest
}
@Test
@Description("Verifies getting multiple actions with the paging request parameter.")
public void getMultipleActionsWithPagingLimitRequestParameter() throws Exception {
final String knownTargetId = "targetId";
final List<Action> actions = generateTargetWithTwoUpdatesWithOneOverride(knownTargetId);
@@ -1208,7 +1202,7 @@ public class MgmtTargetResourceTest extends AbstractManagementApiIntegrationTest
final Map<String, String> knownControllerAttrs = new HashMap<>();
knownControllerAttrs.put("a", "1");
knownControllerAttrs.put("b", "2");
final Target target = testdataFactory.createTarget(knownTargetId);
testdataFactory.createTarget(knownTargetId);
controllerManagament.updateControllerAttributes(knownTargetId, knownControllerAttrs);
// test query target over rest resource
@@ -1221,7 +1215,7 @@ public class MgmtTargetResourceTest extends AbstractManagementApiIntegrationTest
public void getControllerEmptyAttributesReturnsNoContent() throws Exception {
// create target with attributes
final String knownTargetId = "targetIdWithAttributes";
final Target target = testdataFactory.createTarget(knownTargetId);
testdataFactory.createTarget(knownTargetId);
// test query target over rest resource
mvc.perform(get(MgmtRestConstants.TARGET_V1_REQUEST_MAPPING + "/" + knownTargetId + "/attributes"))
@@ -1247,12 +1241,6 @@ public class MgmtTargetResourceTest extends AbstractManagementApiIntegrationTest
+ "\"}]";
}
private String getCreateTargetsListJsonStringWithAdditionalNotExistingAttribute(final String controllerId,
final String name, final String description) {
return "[{\"name\":\"" + name + "\",\"controllerId\":\"" + controllerId + "\",\"description\":\"" + description
+ "\"}]";
}
private Target createSingleTarget(final String controllerId, final String name) {
targetManagement.createTarget(entityFactory.target().create().controllerId(controllerId).name(name)
.description(TARGET_DESCRIPTION_TEST));
@@ -1276,15 +1264,6 @@ public class MgmtTargetResourceTest extends AbstractManagementApiIntegrationTest
}
}
/**
* helper method to give feedback mark an target IN_SYNC
*
*/
private void feedbackToByInSync(final Long actionId) {
controllerManagement
.addUpdateActionStatus(entityFactory.actionStatus().create(actionId).status(Status.FINISHED));
}
/**
* helper method to create a target and start an action on it.
*

View File

@@ -194,7 +194,48 @@ public abstract class JsonBuilder {
new JSONObject().put("cnt", 2).put("of", 5)))
.put("details", messages))
.toString();
}
/**
* Build an invalid request body with missing result for feedback message.
*
* @param id
* id of the action
* @param execution
* the execution
* @param message
* the message
* @return a invalid request body
* @throws JSONException
*/
public static String missingResultInFeedback(final String id, final String execution, final String message)
throws JSONException {
final List<String> messages = new ArrayList<>();
messages.add(message);
return new JSONObject().put("id", id).put("time", "20140511T121314")
.put("status", new JSONObject().put("execution", execution).put("details", messages)).toString();
}
/**
* Build an invalid request body with missing finished result for feedback
* message.
*
* @param id
* id of the action
* @param execution
* the execution
* @param message
* the message
* @return a invalid request body
* @throws JSONException
*/
public static String missingFinishedResultInFeedback(final String id, final String execution, final String message)
throws JSONException {
final List<String> messages = new ArrayList<>();
messages.add(message);
return new JSONObject().put("id", id).put("time", "20140511T121314").put("status",
new JSONObject().put("execution", execution).put("result", new JSONObject()).put("details", messages))
.toString();
}
/**