Skip to content

Commit

Permalink
[JBPM-9611] Command resource returns HTTP OK response even for failure
Browse files Browse the repository at this point in the history 
scenarios
  • Loading branch information
@fjtirado
fjtirado committed Feb 16, 2021
1 parent bdfa3b8 commit fd7891e
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 28 deletions.
33 changes: 31 additions & 2 deletions ...ent/kie-server-remote/kie-server-rest/kie-server-rest-drools/src/build/revapi-config.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,36 @@
"ignores": {
"revapi": {
"_comment": "Changes between 7.48.0.Final and the current branch. These changes are desired and thus ignored.",
"ignore": []
}
"ignore": [
{
"code": "java.annotation.attributeRemoved",
"old": "method javax.ws.rs.core.Response org.kie.server.remote.rest.drools.CommandResource::manageContainer(javax.ws.rs.core.HttpHeaders, java.lang.String, java.lang.String)",
"new": "method javax.ws.rs.core.Response org.kie.server.remote.rest.drools.CommandResource::manageContainer(javax.ws.rs.core.HttpHeaders, java.lang.String, java.lang.String)",
"annotationType": "io.swagger.annotations.ApiOperation",
"annotation": "@io.swagger.annotations.ApiOperation(value = \"Executes one or more runtime commands\", response = org.kie.server.api.model.ServiceResponse.class)",
"attribute": "code",
"value": "200",
"package": "org.kie.server.remote.rest.drools",
"classSimpleName": "CommandResource",
"methodName": "manageContainer",
"elementKind": "method",
"justification": "https://issues.redhat.com/browse/JBPM-9611"
},
{
"code": "java.annotation.attributeValueChanged",
"old": "method javax.ws.rs.core.Response org.kie.server.remote.rest.drools.CommandResource::manageContainer(javax.ws.rs.core.HttpHeaders, java.lang.String, java.lang.String)",
"new": "method javax.ws.rs.core.Response org.kie.server.remote.rest.drools.CommandResource::manageContainer(javax.ws.rs.core.HttpHeaders, java.lang.String, java.lang.String)",
"annotationType": "io.swagger.annotations.ApiResponses",
"annotation": "@io.swagger.annotations.ApiResponses({@io.swagger.annotations.ApiResponse(code = 200, message = \"Successful execution\"), @io.swagger.annotations.ApiResponse(code = 500, message = \"Unexpected error\"), @io.swagger.annotations.ApiResponse(code = 204, message = \"Command execute succesfully, but without response\")})",
"attribute": "value",
"oldValue": "{@io.swagger.annotations.ApiResponse(code = 500, message = \"Unexpected error\")}",
"newValue": "{@io.swagger.annotations.ApiResponse(code = 200, message = \"Successful execution\"), @io.swagger.annotations.ApiResponse(code = 500, message = \"Unexpected error\"), @io.swagger.annotations.ApiResponse(code = 204, message = \"Command execute succesfully, but without response\")}",
"package": "org.kie.server.remote.rest.drools",
"classSimpleName": "CommandResource",
"methodName": "manageContainer",
"elementKind": "method",
"justification": "https://issues.redhat.com/browse/JBPM-9611"
}]
}
}
}
58 changes: 36 additions & 22 deletions ...e-server-rest-drools/src/main/java/org/kie/server/remote/rest/drools/CommandResource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,6 @@

package org.kie.server.remote.rest.drools;

import static org.kie.server.remote.rest.common.util.RestUtils.buildConversationIdHeader;
import static org.kie.server.remote.rest.common.util.RestUtils.createResponse;
import static org.kie.server.remote.rest.common.util.RestUtils.getClassType;
import static org.kie.server.remote.rest.common.util.RestUtils.getContentType;
import static org.kie.server.remote.rest.common.util.RestUtils.getVariant;

import javax.ws.rs.Consumes;
import javax.ws.rs.POST;
import javax.ws.rs.Path;
Expand All @@ -30,8 +24,14 @@
import javax.ws.rs.core.HttpHeaders;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
import javax.ws.rs.core.Response.Status;
import javax.ws.rs.core.Variant;

import io.swagger.annotations.Api;
import io.swagger.annotations.ApiOperation;
import io.swagger.annotations.ApiParam;
import io.swagger.annotations.ApiResponse;
import io.swagger.annotations.ApiResponses;
import org.kie.server.api.marshalling.MarshallingFormat;
import org.kie.server.api.model.ServiceResponse;
import org.kie.server.api.rest.RestURI;
Expand All @@ -43,11 +43,11 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import io.swagger.annotations.Api;
import io.swagger.annotations.ApiOperation;
import io.swagger.annotations.ApiParam;
import io.swagger.annotations.ApiResponse;
import io.swagger.annotations.ApiResponses;
import static org.kie.server.remote.rest.common.util.RestUtils.buildConversationIdHeader;
import static org.kie.server.remote.rest.common.util.RestUtils.createResponse;
import static org.kie.server.remote.rest.common.util.RestUtils.getClassType;
import static org.kie.server.remote.rest.common.util.RestUtils.getContentType;
import static org.kie.server.remote.rest.common.util.RestUtils.getVariant;

@Api(value="KIE session assets")
@Path("server/containers/instances/{" + RestURI.CONTAINER_ID + "}")
Expand All @@ -69,9 +69,10 @@ public CommandResource(KieContainerCommandService delegate, KieServerRegistry re
this.marshallerHelper = new MarshallerHelper(registry);
}

@ApiOperation(value="Executes one or more runtime commands",
response=ServiceResponse.class, code=200)
@ApiResponses(value = { @ApiResponse(code = 500, message = "Unexpected error") })
@ApiOperation(value = "Executes one or more runtime commands", response = ServiceResponse.class)
@ApiResponses({@ApiResponse(code = 200, message = "Successful execution"),
@ApiResponse(code = 500, message = "Unexpected error"),
@ApiResponse(code = 204, message = "Command execute succesfully, but without response")})
@POST
@Consumes({MediaType.APPLICATION_XML, MediaType.APPLICATION_JSON})
@Produces({MediaType.APPLICATION_XML, MediaType.APPLICATION_JSON})
Expand All @@ -88,20 +89,33 @@ public Response manageContainer(@Context HttpHeaders headers,
format = MarshallingFormat.valueOf(contentType);
}
logger.debug("Received request with content '{}'", cmdPayload);
Object result = delegate.callContainer(id, cmdPayload, format, classType);
Header conversationIdHeader = buildConversationIdHeader(id, registry, headers);
@SuppressWarnings("squid:S3740")
ServiceResponse<?> result = delegate.callContainer(id, cmdPayload, format, classType);
Status status;
switch (result.getType()) {
case FAILURE:
status = Response.Status.INTERNAL_SERVER_ERROR;
break;
case NO_RESPONSE:
status = Response.Status.NO_CONTENT;
break;
case SUCCESS:
default:
status = Response.Status.OK;
}
try {
String response = marshallerHelper.marshal(id, format.getType(), result, ContainerLocatorProvider.get().getLocator());
logger.debug("Returning OK response with content '{}'", response);
String response = marshallerHelper.marshal(id, format.getType(), result, ContainerLocatorProvider
.get()
.getLocator());
logger.debug("Returning {} response with content '{}'", status, response);

return createResponse(response, v, Response.Status.OK, conversationIdHeader);
return createResponse(response, v, status, conversationIdHeader);
} catch (IllegalArgumentException e) {
// in case marshalling failed return the call container response to keep backward compatibility
String response = marshallerHelper.marshal(format.getType(), result);
logger.debug("Returning OK response with content '{}'", response);
return createResponse(response, v, Response.Status.OK, conversationIdHeader);
logger.debug("Returning {} response with content '{}'", status, response);
return createResponse(response, v, status, conversationIdHeader);
}

}

}
8 changes: 4 additions & 4 deletions ...java/org/kie/server/integrationtests/drools/rest/RestMalformedRequestIntegrationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@

package org.kie.server.integrationtests.drools.rest;

import static org.junit.Assert.assertEquals;

import javax.ws.rs.client.WebTarget;
import javax.ws.rs.core.GenericType;
import javax.ws.rs.core.Response;
Expand All @@ -35,6 +33,8 @@
import org.kie.server.integrationtests.shared.KieServerDeployer;
import org.kie.server.integrationtests.shared.basetests.RestOnlyBaseIntegrationTest;

import static org.junit.Assert.assertEquals;

@Category(RESTOnly.class)
public class RestMalformedRequestIntegrationTest extends RestOnlyBaseIntegrationTest {

Expand Down Expand Up @@ -62,7 +62,7 @@ public void testInvalidCommandBodyOnCallContainer() throws Exception {

WebTarget clientRequest = newRequest(TestConfig.getKieServerHttpUrl() + "/containers/instances/" + CONTAINER_ID);
response = clientRequest.request(getMediaType()).post(createEntity(body));
assertEquals(Response.Status.OK.getStatusCode(), response.getStatus());
assertEquals(Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), response.getStatus());

ServiceResponse<KieContainerResource> serviceResponse =
response.readEntity(new GenericType<ServiceResponse<KieContainerResource>>(){});
Expand All @@ -87,7 +87,7 @@ public void testInvalidBodyOnCallContainer() throws Exception {

WebTarget clientRequest = newRequest(TestConfig.getKieServerHttpUrl() + "/containers/instances/" + CONTAINER_ID);
response = clientRequest.request(getMediaType()).post(createEntity(body));
assertEquals(Response.Status.OK.getStatusCode(), response.getStatus());
assertEquals(Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), response.getStatus());

ServiceResponse<KieContainerResource> serviceResponse =
response.readEntity(new GenericType<ServiceResponse<KieContainerResource>>(){});
Expand Down

0 comments on commit fd7891e

Please sign in to comment.